Skip to content

Commit

Permalink
Fixes netty#7566 by handling concatenated GZIP streams.
Browse files Browse the repository at this point in the history
Motivation:
According to RFC 1952, concatenation of valid gzip streams is also a valid gzip stream. JdkZlibDecoder only processed the first and discarded the rest.

Modifications:
- Introduced a constructor argument decompressConcatenated that if true, JdkZlibDecoder would continue to process the stream.

Result:
- If 'decompressConcatenated = true', concatenated streams would be processed in
compliance to RFC 1952.
- If 'decompressConcatenated = false' (default), existing behavior would remain.
  • Loading branch information
Abhijit Sarkar authored and normanmaurer committed Jan 17, 2018
1 parent f3c6da3 commit 6ff48dc
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class JdkZlibDecoder extends ZlibDecoder {

// GZIP related
private final ByteBufChecksum crc;
private final boolean decompressConcatenated;

private enum GzipState {
HEADER_START,
Expand All @@ -63,7 +64,7 @@ private enum GzipState {
* Creates a new instance with the default wrapper ({@link ZlibWrapper#ZLIB}).
*/
public JdkZlibDecoder() {
this(ZlibWrapper.ZLIB, null);
this(ZlibWrapper.ZLIB, null, false);
}

/**
Expand All @@ -72,7 +73,7 @@ public JdkZlibDecoder() {
* supports the preset dictionary.
*/
public JdkZlibDecoder(byte[] dictionary) {
this(ZlibWrapper.ZLIB, dictionary);
this(ZlibWrapper.ZLIB, dictionary, false);
}

/**
Expand All @@ -81,13 +82,22 @@ public JdkZlibDecoder(byte[] dictionary) {
* supported atm.
*/
public JdkZlibDecoder(ZlibWrapper wrapper) {
this(wrapper, null);
this(wrapper, null, false);
}

private JdkZlibDecoder(ZlibWrapper wrapper, byte[] dictionary) {
public JdkZlibDecoder(ZlibWrapper wrapper, boolean decompressConcatenated) {
this(wrapper, null, decompressConcatenated);
}

public JdkZlibDecoder(boolean decompressConcatenated) {
this(ZlibWrapper.GZIP, null, decompressConcatenated);
}

private JdkZlibDecoder(ZlibWrapper wrapper, byte[] dictionary, boolean decompressConcatenated) {
if (wrapper == null) {
throw new NullPointerException("wrapper");
}
this.decompressConcatenated = decompressConcatenated;
switch (wrapper) {
case GZIP:
inflater = new Inflater(true);
Expand Down Expand Up @@ -207,7 +217,13 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
if (readFooter) {
gzipState = GzipState.FOOTER_START;
if (readGZIPFooter(in)) {
finished = true;
finished = !decompressConcatenated;

if (!finished) {
inflater.reset();
crc.reset();
gzipState = GzipState.HEADER_START;
}
}
}
} catch (DataFormatException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ public static ZlibDecoder newZlibDecoder() {
if (PlatformDependent.javaVersion() < 7 || noJdkZlibDecoder) {
return new JZlibDecoder();
} else {
return new JdkZlibDecoder();
return new JdkZlibDecoder(true);
}
}

public static ZlibDecoder newZlibDecoder(ZlibWrapper wrapper) {
if (PlatformDependent.javaVersion() < 7 || noJdkZlibDecoder) {
return new JZlibDecoder(wrapper);
} else {
return new JdkZlibDecoder(wrapper);
return new JdkZlibDecoder(wrapper, true);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,20 @@
*/
package io.netty.handler.codec.compression;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.util.CharsetUtil;
import io.netty.util.ReferenceCountUtil;
import org.apache.commons.compress.utils.IOUtils;
import org.junit.Test;

import java.io.IOException;
import java.util.Arrays;
import java.util.Queue;

import static org.junit.Assert.*;


public class JdkZlibTest extends ZlibTest {

Expand All @@ -35,4 +47,47 @@ protected ZlibDecoder createDecoder(ZlibWrapper wrapper) {
public void testZLIB_OR_NONE3() throws Exception {
super.testZLIB_OR_NONE3();
}

@Test
// verifies backward compatibility
public void testConcatenatedStreamsReadFirstOnly() throws IOException {
EmbeddedChannel chDecoderGZip = new EmbeddedChannel(createDecoder(ZlibWrapper.GZIP));

try {
byte[] bytes = IOUtils.toByteArray(getClass().getResourceAsStream("/multiple.gz"));

assertTrue(chDecoderGZip.writeInbound(Unpooled.copiedBuffer(bytes)));
Queue<Object> messages = chDecoderGZip.inboundMessages();
assertEquals(1, messages.size());

ByteBuf msg = (ByteBuf) messages.poll();
assertEquals("a", msg.toString(CharsetUtil.UTF_8));
ReferenceCountUtil.release(msg);
} finally {
assertFalse(chDecoderGZip.finish());
chDecoderGZip.close();
}
}

@Test
public void testConcatenatedStreamsReadFully() throws IOException {
EmbeddedChannel chDecoderGZip = new EmbeddedChannel(new JdkZlibDecoder(true));

try {
byte[] bytes = IOUtils.toByteArray(getClass().getResourceAsStream("/multiple.gz"));

assertTrue(chDecoderGZip.writeInbound(Unpooled.copiedBuffer(bytes)));
Queue<Object> messages = chDecoderGZip.inboundMessages();
assertEquals(2, messages.size());

for (String s : Arrays.asList("a", "b")) {
ByteBuf msg = (ByteBuf) messages.poll();
assertEquals(s, msg.toString(CharsetUtil.UTF_8));
ReferenceCountUtil.release(msg);
}
} finally {
assertFalse(chDecoderGZip.finish());
chDecoderGZip.close();
}
}
}
Binary file added codec/src/test/resources/multiple.gz
Binary file not shown.

0 comments on commit 6ff48dc

Please sign in to comment.