From 02c0c5b5e978cd05327dfcb3c7f87ef12a714a23 Mon Sep 17 00:00:00 2001 From: norman Date: Fri, 6 Jul 2012 13:57:55 +0200 Subject: [PATCH] Revert "Only parse the packet length one time per packet. See #382" as this makes no sense after thinking more about it and just makes it harder to read This reverts commit 784722eff4540a2e81517c1b176f99e422385082. Conflicts: src/main/java/org/jboss/netty/handler/ssl/SslHandler.java --- .../jboss/netty/handler/ssl/SslHandler.java | 127 +++++++++--------- 1 file changed, 60 insertions(+), 67 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java b/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java index 06c5ed3b01..c3a02dbc11 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java +++ b/src/main/java/org/jboss/netty/handler/ssl/SslHandler.java @@ -206,8 +206,6 @@ public class SslHandler extends FrameDecoder private final SSLEngineInboundCloseFuture sslEngineCloseFuture = new SSLEngineInboundCloseFuture(); - private int packetLength = -1; - /** * Creates a new instance. * @@ -636,74 +634,73 @@ public class SslHandler extends FrameDecoder protected Object decode( final ChannelHandlerContext ctx, Channel channel, ChannelBuffer buffer) throws Exception { - if (packetLength == -1) { - if (buffer.readableBytes() < 5) { - return null; - } + if (buffer.readableBytes() < 5) { + return null; + } - // SSLv3 or TLS - Check ContentType - boolean tls; - switch (buffer.getUnsignedByte(buffer.readerIndex())) { - case 20: // change_cipher_spec - case 21: // alert - case 22: // handshake - case 23: // application_data - tls = true; - break; - default: - // SSLv2 or bad data - tls = false; - } + int packetLength = 0; - if (tls) { - // SSLv3 or TLS - Check ProtocolVersion - int majorVersion = buffer.getUnsignedByte(buffer.readerIndex() + 1); - if (majorVersion == 3) { - // SSLv3 or TLS - packetLength = (getShort(buffer, buffer.readerIndex() + 3) & 0xFFFF) + 5; - if (packetLength <= 5) { - // Neither SSLv3 or TLSv1 (i.e. SSLv2 or bad data) - tls = false; - } - } else { + // SSLv3 or TLS - Check ContentType + boolean tls; + switch (buffer.getUnsignedByte(buffer.readerIndex())) { + case 20: // change_cipher_spec + case 21: // alert + case 22: // handshake + case 23: // application_data + tls = true; + break; + default: + // SSLv2 or bad data + tls = false; + } + + if (tls) { + // SSLv3 or TLS - Check ProtocolVersion + int majorVersion = buffer.getUnsignedByte(buffer.readerIndex() + 1); + if (majorVersion == 3) { + // SSLv3 or TLS + packetLength = (getShort(buffer, buffer.readerIndex() + 3) & 0xFFFF) + 5; + if (packetLength <= 5) { // Neither SSLv3 or TLSv1 (i.e. SSLv2 or bad data) tls = false; } + } else { + // Neither SSLv3 or TLSv1 (i.e. SSLv2 or bad data) + tls = false; } - - if (!tls) { - // SSLv2 or bad data - Check the version - boolean sslv2 = true; - int headerLength = (buffer.getUnsignedByte( - buffer.readerIndex()) & 0x80) != 0 ? 2 : 3; - int majorVersion = buffer.getUnsignedByte( - buffer.readerIndex() + headerLength + 1); - if (majorVersion == 2 || majorVersion == 3) { - // SSLv2 - if (headerLength == 2) { - packetLength = (getShort(buffer, buffer.readerIndex()) & 0x7FFF) + 2; - } else { - packetLength = (getShort(buffer, buffer.readerIndex()) & 0x3FFF) + 3; - } - if (packetLength <= headerLength) { - sslv2 = false; - } - } else { - sslv2 = false; - } - - if (!sslv2) { - // Bad data - discard the buffer and raise an exception. - NotSslRecordException e = new NotSslRecordException( - "not an SSL/TLS record: " + ChannelBuffers.hexDump(buffer)); - buffer.skipBytes(buffer.readableBytes()); - throw e; - } - } - - assert packetLength > 0; } + if (!tls) { + // SSLv2 or bad data - Check the version + boolean sslv2 = true; + int headerLength = (buffer.getUnsignedByte( + buffer.readerIndex()) & 0x80) != 0 ? 2 : 3; + int majorVersion = buffer.getUnsignedByte( + buffer.readerIndex() + headerLength + 1); + if (majorVersion == 2 || majorVersion == 3) { + // SSLv2 + if (headerLength == 2) { + packetLength = (getShort(buffer, buffer.readerIndex()) & 0x7FFF) + 2; + } else { + packetLength = (getShort(buffer, buffer.readerIndex()) & 0x3FFF) + 3; + } + if (packetLength <= headerLength) { + sslv2 = false; + } + } else { + sslv2 = false; + } + + if (!sslv2) { + // Bad data - discard the buffer and raise an exception. + NotSslRecordException e = new NotSslRecordException( + "not an SSL/TLS record: " + ChannelBuffers.hexDump(buffer)); + buffer.skipBytes(buffer.readableBytes()); + throw e; + } + } + + assert packetLength > 0; if (buffer.readableBytes() < packetLength) { return null; @@ -725,11 +722,7 @@ public class SslHandler extends FrameDecoder // before calling the user code. final int packetOffset = buffer.readerIndex(); buffer.skipBytes(packetLength); - try { - return unwrap(ctx, channel, buffer, packetOffset, packetLength); - } finally { - packetLength = -1; - } + return unwrap(ctx, channel, buffer, packetOffset, packetLength); } /**