Add ZLIB_OR_NONE support to JdkZlibDecoder [netty#2016]

buchgr · Mar 1, 2014 · a0ed180 · a0ed180
1 parent b6aa032
commit a0ed180
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 42 deletions.
diff --git a/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibDecoder.java b/codec/src/main/java/io/netty/handler/codec/compression/JdkZlibDecoder.java
@@ -35,7 +35,7 @@ public class JdkZlibDecoder extends ZlibDecoder {
     private static final int FCOMMENT = 0x10;
     private static final int FRESERVED = 0xE0;
 
-    private final Inflater inflater;
+    private Inflater inflater;
     private final byte[] dictionary;
 
     // GZIP related
@@ -58,6 +58,8 @@ private enum GzipState {
 
     private volatile boolean finished;
 
+    private boolean decideZlibOrNone;
+
     /**
      * Creates a new instance with the default wrapper ({@link ZlibWrapper#ZLIB}).
      */
@@ -100,6 +102,11 @@ private JdkZlibDecoder(ZlibWrapper wrapper, byte[] dictionary) {
                 inflater = new Inflater();
                 crc = null;
                 break;
+            case ZLIB_OR_NONE:
+                // Postpone the decision until decode(...) is called.
+                decideZlibOrNone = true;
+                crc = null;
+                break;
             default:
                 throw new IllegalArgumentException("Only GZIP or ZLIB is supported, but you used " + wrapper);
         }
@@ -123,6 +130,17 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
             return;
         }
 
+        if (decideZlibOrNone) {
+            // First two bytes are needed to decide if it's a ZLIB stream.
+            if (in.readableBytes() < 2) {
+                return;
+            }
+
+            boolean nowrap = !looksLikeZlib(in.getShort(0));
+            inflater = new Inflater(nowrap);
+            decideZlibOrNone = false;
+        }
+
         if (crc != null) {
             switch (gzipState) {
                 case FOOTER_START:
@@ -214,7 +232,9 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
     @Override
     protected void handlerRemoved0(ChannelHandlerContext ctx) throws Exception {
         super.handlerRemoved0(ctx);
-        inflater.end();
+        if (inflater != null) {
+            inflater.end();
+        }
     }
 
     private boolean readGZIPHeader(ByteBuf in) {
@@ -356,4 +376,16 @@ private void verifyCrc(ByteBuf in) {
                     "CRC value missmatch. Expected: " + crcValue + ", Got: " + readCrc);
         }
     }
+
+    /*
+     * Returns true if the cmf_flg parameter (think: first two bytes of a zlib stream)
+     * indicates that this is a zlib stream.
+     * <p>
+     * You can lookup the details in the ZLIB RFC:
+     * <a href="http://tools.ietf.org/html/rfc1950#section-2.2">RFC 1950</a>.
+     */
+    private boolean looksLikeZlib(short cmf_flg) {
+        return (cmf_flg & 0x7800) == 0x7800 &&
+                cmf_flg % 31 == 0;
+    }
 }
diff --git a/codec/src/main/java/io/netty/handler/codec/compression/ZlibCodecFactory.java b/codec/src/main/java/io/netty/handler/codec/compression/ZlibCodecFactory.java
@@ -98,15 +98,10 @@ public static ZlibDecoder newZlibDecoder() {
     }
 
     public static ZlibDecoder newZlibDecoder(ZlibWrapper wrapper) {
-        switch (wrapper) {
-            case ZLIB_OR_NONE:
-                return new JZlibDecoder(wrapper);
-            default:
-                if (PlatformDependent.javaVersion() < 7 || noJdkZlibDecoder) {
-                    return new JZlibDecoder(wrapper);
-                } else {
-                    return new JdkZlibDecoder(wrapper);
-                }
+        if (PlatformDependent.javaVersion() < 7 || noJdkZlibDecoder) {
+            return new JZlibDecoder(wrapper);
+        } else {
+            return new JdkZlibDecoder(wrapper);
         }
     }
 

diff --git a/codec/src/test/java/io/netty/handler/codec/compression/JdkZlibTest.java b/codec/src/test/java/io/netty/handler/codec/compression/JdkZlibTest.java
@@ -30,21 +30,8 @@ protected ZlibDecoder createDecoder(ZlibWrapper wrapper) {
         return new JdkZlibDecoder(wrapper);
     }
 
-    @Override
-    @Test(expected = IllegalArgumentException.class)
-    public void testZLIB_OR_NONE() throws Exception {
-        new JdkZlibDecoder(ZlibWrapper.ZLIB_OR_NONE);
-    }
-
-    @Override
-    @Test(expected = IllegalArgumentException.class)
-    public void testZLIB_OR_NONE2() throws Exception {
-        new JdkZlibDecoder(ZlibWrapper.ZLIB_OR_NONE);
-    }
-
-    @Override
-    @Test(expected = IllegalArgumentException.class)
+    @Test(expected = DecompressionException.class)
     public void testZLIB_OR_NONE3() throws Exception {
-        new JdkZlibDecoder(ZlibWrapper.ZLIB_OR_NONE);
+        super.testZLIB_OR_NONE3();
     }
 }
diff --git a/codec/src/test/java/io/netty/handler/codec/compression/ZlibCrossTest2.java b/codec/src/test/java/io/netty/handler/codec/compression/ZlibCrossTest2.java
@@ -29,21 +29,8 @@ protected ZlibDecoder createDecoder(ZlibWrapper wrapper) {
         return new JdkZlibDecoder(wrapper);
     }
 
-    @Override
-    @Test(expected = IllegalArgumentException.class)
-    public void testZLIB_OR_NONE() throws Exception {
-        new JdkZlibDecoder(ZlibWrapper.ZLIB_OR_NONE);
-    }
-
-    @Override
-    @Test(expected = IllegalArgumentException.class)
-    public void testZLIB_OR_NONE2() throws Exception {
-        new JdkZlibDecoder(ZlibWrapper.ZLIB_OR_NONE);
-    }
-
-    @Override
-    @Test(expected = IllegalArgumentException.class)
+    @Test(expected = DecompressionException.class)
     public void testZLIB_OR_NONE3() throws Exception {
-        new JdkZlibDecoder(ZlibWrapper.ZLIB_OR_NONE);
+        super.testZLIB_OR_NONE3();
     }
 }
diff --git a/codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java b/codec/src/test/java/io/netty/handler/codec/compression/ZlibTest.java
@@ -179,7 +179,7 @@ public void testZLIB_OR_NONE() throws Exception {
     public void testZLIB_OR_NONE2() throws Exception {
         testCompressNone(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB_OR_NONE);
         testCompressSmall(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB_OR_NONE);
-        testCompressLarge(ZlibWrapper.GZIP, ZlibWrapper.ZLIB_OR_NONE);
+        testCompressLarge(ZlibWrapper.ZLIB, ZlibWrapper.ZLIB_OR_NONE);
     }
 
     @Test