Fix NPE problems

Motivation:

Now Netty has a few problems with null values.

Modifications:

- Check HAProxyProxiedProtocol in HAProxyMessage constructor and throw NPE if it is null.
If HAProxyProxiedProtocol is null we will set AddressFamily as null. So we will get NPE inside checkAddress(String, AddressFamily) and it won't be easy to understand why addrFamily is null.
- Check File in DiskFileUpload.toString().
If File is null we will get NPE when calling toString() method.
- Check Result<String> in MqttDecoder.decodeConnectionPayload(...).
If !mqttConnectVariableHeader.isWillFlag() || !mqttConnectVariableHeader.hasUserName() || !mqttConnectVariableHeader.hasPassword() we will get NPE when we will try to create new instance of MqttConnectPayload.
- 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 check in DefaultStompFrame(StompCommand) constructor.
Because we have this check in the super class.
- 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 a5095c5616
commit 6860019f41
12 changed files with 31 additions and 47 deletions

View File

@ -74,12 +74,10 @@ public final class HAProxyMessage {
HAProxyProtocolVersion protocolVersion, HAProxyCommand command, HAProxyProxiedProtocol proxiedProtocol,
String sourceAddress, String destinationAddress, int sourcePort, int destinationPort) {
AddressFamily addrFamily;
if (proxiedProtocol != null) {
addrFamily = proxiedProtocol.addressFamily();
} else {
addrFamily = null;
if (proxiedProtocol == null) {
throw new NullPointerException("proxiedProtocol");
}
AddressFamily addrFamily = proxiedProtocol.addressFamily();
checkAddress(sourceAddress, addrFamily);
checkAddress(destinationAddress, addrFamily);

View File

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

View File

@ -54,7 +54,7 @@ public class CloseWebSocketFrame extends WebSocketFrame {
* reserved bits used for protocol extensions
*/
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;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.codec.MessageToMessageEncoder;
import io.netty.handler.codec.TooLongFrameException;
@ -96,14 +95,9 @@ public class WebSocket08FrameEncoder extends MessageToMessageEncoder<WebSocketFr
@Override
protected void encode(ChannelHandlerContext ctx, WebSocketFrame msg, List<Object> out) throws Exception {
final ByteBuf data = msg.content();
byte[] mask;
ByteBuf data = msg.content();
if (data == null) {
data = Unpooled.EMPTY_BUFFER;
}
byte opcode;
if (msg instanceof TextWebSocketFrame) {
opcode = OPCODE_TEXT;

View File

@ -348,10 +348,10 @@ public class MqttDecoder extends ReplayingDecoder<DecoderState> {
final MqttConnectPayload mqttConnectPayload =
new MqttConnectPayload(
decodedClientId.value,
decodedWillTopic.value,
decodedWillMessage.value,
decodedUserName.value,
decodedPassword.value);
decodedWillTopic != null ? decodedWillTopic.value : null,
decodedWillMessage != null ? decodedWillMessage.value : null,
decodedUserName != null ? decodedUserName.value : null,
decodedPassword != null ? decodedPassword.value : null);
return new Result<MqttConnectPayload>(mqttConnectPayload, numberOfBytesConsumed);
}

View File

@ -28,9 +28,6 @@ public class DefaultStompFrame extends DefaultStompHeadersSubframe implements St
public DefaultStompFrame(StompCommand command) {
this(command, Unpooled.buffer(0));
if (command == null) {
throw new NullPointerException("command");
}
}
public DefaultStompFrame(StompCommand command, ByteBuf content) {

View File

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

View File

@ -80,16 +80,17 @@ final class PlatformDependent0 {
Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe");
unsafeField.setAccessible(true);
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.
// https://github.com/netty/netty/issues/1061
// http://www.mail-archive.com/jdk6-dev@openjdk.java.net/msg00698.html
try {
unsafe.getClass().getDeclaredMethod(
"copyMemory", Object.class, long.class, Object.class, long.class, long.class);
logger.debug("sun.misc.Unsafe.copyMemory: available");
if (unsafe != null) {
unsafe.getClass().getDeclaredMethod(
"copyMemory", Object.class, long.class, Object.class, long.class, long.class);
logger.debug("sun.misc.Unsafe.copyMemory: available");
}
} catch (NoSuchMethodError t) {
logger.debug("sun.misc.Unsafe.copyMemory: unavailable");
throw t;

View File

@ -2830,14 +2830,14 @@ public class ConcurrentHashMapV8<K,V>
else
sp.right = p;
}
if ((s.right = pr) != null)
pr.parent = s;
s.right = pr;
pr.parent = s;
}
p.left = null;
s.left = pl;
pl.parent = s;
if ((p.right = sr) != null)
sr.parent = p;
if ((s.left = pl) != null)
pl.parent = s;
if ((s.parent = pp) == null)
r = s;
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.
*/
public void setTicketKeys(byte[] keys) {
if (keys != null) {
if (keys == null) {
throw new NullPointerException("keys");
}
SSLContext.setSessionTicketKeys(ctx, keys);

View File

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

View File

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