Fix NPE problems

Motivation:

Now Netty has a few problems with null values.

Modifications:

- Check File in DiskFileUpload.toString().
If File is null we will get NPE when calling toString() method.
- Check Result<String> in MqttDecoder.decodeConnectionPayload(...).
- Check Unsafe before calling unsafe.getClass() in PlatformDependent0 static block.
- Removed unnecessary null check in WebSocket08FrameEncoder.encode(...).
Because msg.content() can not return null.
- Removed unnecessary null checks in ConcurrentHashMapV8.removeTreeNode(TreeNode<K,V>).
- Removed unnecessary null check in OioDatagramChannel.doReadMessages(List<Object>).
Because tmpPacket.getSocketAddress() always returns new SocketAddress instance.
- Removed unnecessary null check in OioServerSocketChannel.doReadMessages(List<Object>).
Because socket.accept() always returns new Socket instance.
- Pass Unpooled.buffer(0) instead of null inside CloseWebSocketFrame(boolean, int) constructor.
If we will pass null we will get NPE in super class constructor.
- Added throw new IllegalStateException in GlobalEventExecutor.awaitInactivity(long, TimeUnit) if it will be called before GlobalEventExecutor.execute(Runnable).
Because now we will get NPE. IllegalStateException will be better in this case.
- Fixed null check in OpenSslServerContext.setTicketKeys(byte[]).
Now we throw new NPE if byte[] is not null.

Result:

Added new null checks when it is necessary, removed unnecessary null checks and fixed some NPE problems.
This commit is contained in:
Idel Pivnitskiy 2014-07-19 21:51:19 +04:00 committed by Norman Maurer
parent 13569481bf
commit dd026eb60a
9 changed files with 24 additions and 35 deletions

View File

@ -133,7 +133,7 @@ public class DiskFileUpload extends AbstractDiskHttpData implements FileUpload {
HttpHeaders.Names.CONTENT_LENGTH + ": " + length() + "\r\n" + HttpHeaders.Names.CONTENT_LENGTH + ": " + length() + "\r\n" +
"Completed: " + isCompleted() + "Completed: " + isCompleted() +
"\r\nIsInMemory: " + isInMemory() + "\r\nRealFile: " + "\r\nIsInMemory: " + isInMemory() + "\r\nRealFile: " +
file.getAbsolutePath() + " DefaultDeleteAfter: " + (file != null ? file.getAbsolutePath() : "null") + " DefaultDeleteAfter: " +
deleteOnExitTemporaryFile; deleteOnExitTemporaryFile;
} }

View File

@ -54,7 +54,7 @@ public class CloseWebSocketFrame extends WebSocketFrame {
* reserved bits used for protocol extensions * reserved bits used for protocol extensions
*/ */
public CloseWebSocketFrame(boolean finalFragment, int rsv) { public CloseWebSocketFrame(boolean finalFragment, int rsv) {
this(finalFragment, rsv, null); this(finalFragment, rsv, Unpooled.buffer(0));
} }
/** /**

View File

@ -54,7 +54,6 @@
package io.netty.handler.codec.http.websocketx; package io.netty.handler.codec.http.websocketx;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.MessageToMessageEncoder; import io.netty.handler.codec.MessageToMessageEncoder;
import io.netty.handler.codec.TooLongFrameException; import io.netty.handler.codec.TooLongFrameException;
@ -96,14 +95,9 @@ public class WebSocket08FrameEncoder extends MessageToMessageEncoder<WebSocketFr
@Override @Override
protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object> out) throws Exception { protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object> out) throws Exception {
final ByteBuf data = msg.content();
byte[] mask; byte[] mask;
ByteBuf data = msg.content();
if (data == null) {
data = Unpooled.EMPTY_BUFFER;
}
byte opcode; byte opcode;
if (msg instanceof TextWebSocketFrame) { if (msg instanceof TextWebSocketFrame) {
opcode = OPCODE_TEXT; opcode = OPCODE_TEXT;

View File

@ -203,10 +203,11 @@ public final class GlobalEventExecutor extends AbstractEventExecutor {
throw new NullPointerException("unit"); throw new NullPointerException("unit");
} }
Thread thread = this.thread; final Thread thread = this.thread;
if (thread != null) { if (thread == null) {
thread.join(unit.toMillis(timeout)); throw new IllegalStateException("thread was not started");
} }
thread.join(unit.toMillis(timeout));
return !thread.isAlive(); return !thread.isAlive();
} }

View File

@ -80,16 +80,17 @@ final class PlatformDependent0 {
Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe"); Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe");
unsafeField.setAccessible(true); unsafeField.setAccessible(true);
unsafe = (Unsafe) unsafeField.get(null); unsafe = (Unsafe) unsafeField.get(null);
logger.debug("sun.misc.Unsafe.theUnsafe: {}", unsafe != null? "available" : "unavailable"); logger.debug("sun.misc.Unsafe.theUnsafe: {}", unsafe != null ? "available" : "unavailable");
// Ensure the unsafe supports all necessary methods to work around the mistake in the latest OpenJDK. // Ensure the unsafe supports all necessary methods to work around the mistake in the latest OpenJDK.
// https://github.com/netty/netty/issues/1061 // https://github.com/netty/netty/issues/1061
// http://www.mail-archive.com/jdk6-dev@openjdk.java.net/msg00698.html // http://www.mail-archive.com/jdk6-dev@openjdk.java.net/msg00698.html
try { try {
if (unsafe != null) {
unsafe.getClass().getDeclaredMethod( unsafe.getClass().getDeclaredMethod(
"copyMemory", Object.class, long.class, Object.class, long.class, long.class); "copyMemory", Object.class, long.class, Object.class, long.class, long.class);
logger.debug("sun.misc.Unsafe.copyMemory: available"); logger.debug("sun.misc.Unsafe.copyMemory: available");
}
} catch (NoSuchMethodError t) { } catch (NoSuchMethodError t) {
logger.debug("sun.misc.Unsafe.copyMemory: unavailable"); logger.debug("sun.misc.Unsafe.copyMemory: unavailable");
throw t; throw t;

View File

@ -2830,14 +2830,14 @@ public class ConcurrentHashMapV8<K,V>
else else
sp.right = p; sp.right = p;
} }
if ((s.right = pr) != null) s.right = pr;
pr.parent = s; pr.parent = s;
} }
p.left = null; p.left = null;
s.left = pl;
pl.parent = s;
if ((p.right = sr) != null) if ((p.right = sr) != null)
sr.parent = p; sr.parent = p;
if ((s.left = pl) != null)
pl.parent = s;
if ((s.parent = pp) == null) if ((s.parent = pp) == null)
r = s; r = s;
else if (p == pp.left) else if (p == pp.left)

View File

@ -322,7 +322,7 @@ public final class OpenSslServerContext extends SslContext {
* Sets the SSL session ticket keys of this context. * Sets the SSL session ticket keys of this context.
*/ */
public void setTicketKeys(byte[] keys) { public void setTicketKeys(byte[] keys) {
if (keys != null) { if (keys == null) {
throw new NullPointerException("keys"); throw new NullPointerException("keys");
} }
SSLContext.setSessionTicketKeys(ctx, keys); SSLContext.setSessionTicketKeys(ctx, keys);

View File

@ -209,9 +209,6 @@ public class OioDatagramChannel extends AbstractOioMessageChannel
socket.receive(tmpPacket); socket.receive(tmpPacket);
InetSocketAddress remoteAddr = (InetSocketAddress) tmpPacket.getSocketAddress(); InetSocketAddress remoteAddr = (InetSocketAddress) tmpPacket.getSocketAddress();
if (remoteAddr == null) {
remoteAddr = remoteAddress();
}
int readBytes = tmpPacket.getLength(); int readBytes = tmpPacket.getLength();
allocHandle.record(readBytes); allocHandle.record(readBytes);

View File

@ -153,20 +153,16 @@ public class OioServerSocketChannel extends AbstractOioMessageChannel
try { try {
Socket s = socket.accept(); Socket s = socket.accept();
try { try {
if (s != null) {
buf.add(new OioSocketChannel(this, s)); buf.add(new OioSocketChannel(this, s));
return 1; return 1;
}
} catch (Throwable t) { } catch (Throwable t) {
logger.warn("Failed to create a new channel from an accepted socket.", t); logger.warn("Failed to create a new channel from an accepted socket.", t);
if (s != null) {
try { try {
s.close(); s.close();
} catch (Throwable t2) { } catch (Throwable t2) {
logger.warn("Failed to close a socket.", t2); logger.warn("Failed to close a socket.", t2);
} }
} }
}
} catch (SocketTimeoutException e) { } catch (SocketTimeoutException e) {
// Expected // Expected
} }