From f059d8da03348cd81f0deb5900aa42e22f86fbee Mon Sep 17 00:00:00 2001 From: Ryley Kimmel Date: Fri, 27 Feb 2015 14:22:26 -0500 Subject: [PATCH 1/3] Fix resource leak and unnecessary recursion when invalid requests are received. --- src/org/apollo/net/ApolloHandler.java | 32 +++++++--- .../net/codec/handshake/HandshakeDecoder.java | 61 +++++++++++-------- 2 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/org/apollo/net/ApolloHandler.java b/src/org/apollo/net/ApolloHandler.java index ad51eb83..d6b00b91 100644 --- a/src/org/apollo/net/ApolloHandler.java +++ b/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 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); - } else { + session = new UpdateSession(ctx.channel(), serverContext); + } + + // 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); } } diff --git a/src/org/apollo/net/codec/handshake/HandshakeDecoder.java b/src/org/apollo/net/codec/handshake/HandshakeDecoder.java index a0bf1020..b4f7f111 100644 --- a/src/org/apollo/net/codec/handshake/HandshakeDecoder.java +++ b/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 out) { - if (buffer.isReadable()) { - 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); - 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())); - } + protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List out) throws Exception { + if (!buffer.isReadable()) { + return; } + + 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)); } } \ No newline at end of file From 39ccf3ae2bd76701b855baef9e6d320db2874b61 Mon Sep 17 00:00:00 2001 From: Ryley Kimmel Date: Fri, 27 Feb 2015 14:26:04 -0500 Subject: [PATCH 2/3] Fix typo and remove thrown exception. --- src/org/apollo/net/ApolloHandler.java | 8 ++++---- src/org/apollo/net/codec/handshake/HandshakeDecoder.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/org/apollo/net/ApolloHandler.java b/src/org/apollo/net/ApolloHandler.java index d6b00b91..91a12c80 100644 --- a/src/org/apollo/net/ApolloHandler.java +++ b/src/org/apollo/net/ApolloHandler.java @@ -72,15 +72,15 @@ public final class ApolloHandler extends ChannelInboundHandlerAdapter { Session session = attr.get(); + if (message instanceof HttpRequest || message instanceof JagGrabRequest) { + session = new UpdateSession(ctx.channel(), serverContext); + } + if (session != null) { session.messageReceived(message); return; } - if (message instanceof HttpRequest || message instanceof JagGrabRequest) { - session = new UpdateSession(ctx.channel(), serverContext); - } - // TODO: Perhaps let HandshakeMessage implement Message to remove this explicit check if (message instanceof HandshakeMessage) { HandshakeMessage handshakeMessage = (HandshakeMessage) message; diff --git a/src/org/apollo/net/codec/handshake/HandshakeDecoder.java b/src/org/apollo/net/codec/handshake/HandshakeDecoder.java index b4f7f111..31da920c 100644 --- a/src/org/apollo/net/codec/handshake/HandshakeDecoder.java +++ b/src/org/apollo/net/codec/handshake/HandshakeDecoder.java @@ -26,7 +26,7 @@ public final class HandshakeDecoder extends ByteToMessageDecoder { private static final Logger logger = Logger.getLogger(HandshakeDecoder.class.getName()); @Override - protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List out) throws Exception { + protected void decode(ChannelHandlerContext ctx, ByteBuf buffer, List out) { if (!buffer.isReadable()) { return; } @@ -49,7 +49,7 @@ public final class HandshakeDecoder extends ByteToMessageDecoder { default: ByteBuf data = buffer.readBytes(buffer.readableBytes()); - logger.info(String.format("Unecpected handshake request received: %d data: %s", id, data.toString())); + logger.info(String.format("Unexpected handshake request received: %d data: %s", id, data.toString())); return; } From bb2ef1435f674ba312844c72f275fb45666068da Mon Sep 17 00:00:00 2001 From: Ryley Kimmel Date: Fri, 27 Feb 2015 14:26:59 -0500 Subject: [PATCH 3/3] Rename attr -> attribute, remove unnecessary whitespace. --- src/org/apollo/net/ApolloHandler.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/org/apollo/net/ApolloHandler.java b/src/org/apollo/net/ApolloHandler.java index 91a12c80..ce0adfc9 100644 --- a/src/org/apollo/net/ApolloHandler.java +++ b/src/org/apollo/net/ApolloHandler.java @@ -68,9 +68,8 @@ public final class ApolloHandler extends ChannelInboundHandlerAdapter { @Override public void channelRead(ChannelHandlerContext ctx, Object message) { try { - Attribute attr = ctx.attr(NetworkConstants.SESSION_KEY); - - Session session = attr.get(); + Attribute attribute = ctx.attr(NetworkConstants.SESSION_KEY); + Session session = attribute.get(); if (message instanceof HttpRequest || message instanceof JagGrabRequest) { session = new UpdateSession(ctx.channel(), serverContext); @@ -87,11 +86,11 @@ public final class ApolloHandler extends ChannelInboundHandlerAdapter { switch (handshakeMessage.getServiceId()) { case HandshakeConstants.SERVICE_GAME: - attr.set(new LoginSession(ctx, serverContext)); + attribute.set(new LoginSession(ctx, serverContext)); break; case HandshakeConstants.SERVICE_UPDATE: - attr.set(new UpdateSession(ctx.channel(), serverContext)); + attribute.set(new UpdateSession(ctx.channel(), serverContext)); break; } }