Fix resource leak and unnecessary recursion when invalid requests are received.

src/org/apollo/net/ApolloHandler.java

@@ -5,6 +5,8 @@ import io.netty.channel.ChannelHandler.Sharable;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.handler.codec.http.HttpRequest;
+import io.netty.util.Attribute;
+import io.netty.util.ReferenceCountUtil;
 
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -65,25 +67,37 @@ public final class ApolloHandler extends ChannelInboundHandlerAdapter {
 
 	@Override
 	public void channelRead(ChannelHandlerContext ctx, Object message) {
-		if (ctx.attr(NetworkConstants.SESSION_KEY).get() == null) {
+		try {
+			Attribute<Session> attr = ctx.attr(NetworkConstants.SESSION_KEY);
+
+			Session session = attr.get();
+
+			if (session != null) {
+				session.messageReceived(message);
+				return;
+			}
+
 			if (message instanceof HttpRequest || message instanceof JagGrabRequest) {
-				new UpdateSession(ctx.channel(), serverContext).messageReceived(message);
+				session = new UpdateSession(ctx.channel(), serverContext);
-			} else {
+			}
+
+			// TODO: Perhaps let HandshakeMessage implement Message to remove this explicit check
+			if (message instanceof HandshakeMessage) {
 				HandshakeMessage handshakeMessage = (HandshakeMessage) message;
 
 				switch (handshakeMessage.getServiceId()) {
 				case HandshakeConstants.SERVICE_GAME:
-					ctx.attr(NetworkConstants.SESSION_KEY).set(new LoginSession(ctx, serverContext));
+					attr.set(new LoginSession(ctx, serverContext));
 					break;
+
 				case HandshakeConstants.SERVICE_UPDATE:
-					ctx.attr(NetworkConstants.SESSION_KEY).set(new UpdateSession(ctx.channel(), serverContext));
+					attr.set(new UpdateSession(ctx.channel(), serverContext));
 					break;
-				default:
-					throw new IllegalStateException("Invalid service id.");
 				}
 			}
-		} else {
+
-			ctx.attr(NetworkConstants.SESSION_KEY).get().messageReceived(message);
+		} finally {
+			ReferenceCountUtil.release(message);
 		}
 	}
 

src/org/apollo/net/codec/handshake/HandshakeDecoder.java

@@ -5,6 +5,7 @@ import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
 
 import java.util.List;
+import java.util.logging.Logger;
 
 import org.apollo.net.codec.login.LoginDecoder;
 import org.apollo.net.codec.login.LoginEncoder;
@@ -19,35 +20,41 @@ import org.apollo.net.codec.update.UpdateEncoder;
  */
 public final class HandshakeDecoder extends ByteToMessageDecoder {
 
+	/**
+	 * The logger for this class.
+	 */
+	private static final Logger logger = Logger.getLogger(HandshakeDecoder.class.getName());
+
 	@Override
-	protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> out) {
+	protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List<Object> out) throws Exception {
-		if (buffer.isReadable()) {
+		if (!buffer.isReadable()) {
-			int id = buffer.readUnsignedByte();
+			return;
-
-			switch (id) {
-			case HandshakeConstants.SERVICE_GAME:
-				ctx.pipeline().addFirst("loginEncoder", new LoginEncoder());
-				ctx.pipeline().addAfter("handshakeDecoder", "loginDecoder", new LoginDecoder());
-				break;
-			case HandshakeConstants.SERVICE_UPDATE:
-				ctx.pipeline().addFirst("updateEncoder", new UpdateEncoder());
-				ctx.pipeline().addBefore("handler", "updateDecoder", new UpdateDecoder());
-				ByteBuf buf = ctx.alloc().buffer(8);
-				buf.writeLong(0);
-				ctx.channel().writeAndFlush(buf);
-				break;
-			default:
-				throw new IllegalArgumentException("Invalid service id.");
-			}
-
-			ctx.pipeline().remove(this);
-			HandshakeMessage message = new HandshakeMessage(id);
-
-			out.add(message);
-			if (buffer.isReadable()) {
-				out.add(buffer.readBytes(buffer.readableBytes()));
-			}
 		}
+
+		int id = buffer.readUnsignedByte();
+
+		switch (id) {
+		case HandshakeConstants.SERVICE_GAME:
+			ctx.pipeline().addFirst("loginEncoder", new LoginEncoder());
+			ctx.pipeline().addAfter("handshakeDecoder", "loginDecoder", new LoginDecoder());
+			break;
+
+		case HandshakeConstants.SERVICE_UPDATE:
+			ctx.pipeline().addFirst("updateEncoder", new UpdateEncoder());
+			ctx.pipeline().addBefore("handler", "updateDecoder", new UpdateDecoder());
+
+			ByteBuf buf = ctx.alloc().buffer(8).writeLong(0);
+			ctx.channel().writeAndFlush(buf);
+			break;
+
+		default:
+			ByteBuf data = buffer.readBytes(buffer.readableBytes());
+			logger.info(String.format("Unecpected handshake request received: %d data: %s", id, data.toString()));
+			return;
+		}
+
+		ctx.pipeline().remove(this);
+		out.add(new HandshakeMessage(id));
 	}
 
 }