From 2b58379f88899867484bdb94d82e7ace47edf8dc Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 11 Dec 2019 14:38:42 -0600 Subject: [PATCH] Issue #4415 - Addressing Gzip Decoding of large files + Now applying proper RFC 1952 ISIZE check. + Bit shifting is done with Longs against Long value. Signed-off-by: Joakim Erdfelt --- .../jetty/http/GZIPContentDecoder.java | 10 +- .../jetty/http/GZIPContentDecoderTest.java | 100 ++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index 6ca334aba29..c8d7af6f2e0 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -37,13 +37,16 @@ import org.eclipse.jetty.util.component.Destroyable; */ public class GZIPContentDecoder implements Destroyable { + // Unsigned Integer Max == 2^32 + private static final long UINT_MAX = 0xffffffffL; + private final List _inflateds = new ArrayList<>(); private final Inflater _inflater = new Inflater(true); private final ByteBufferPool _pool; private final int _bufferSize; private State _state; private int _size; - private int _value; + private long _value; private byte _flags; private ByteBuffer _inflated; @@ -375,11 +378,12 @@ public class GZIPContentDecoder implements Destroyable } case ISIZE: { - _value += (currByte & 0xFF) << 8 * _size; + _value = _value | ((currByte & 0xFFL) << (8 * _size)); ++_size; if (_size == 4) { - if (_value != _inflater.getBytesWritten()) + // RFC 1952: Section 2.3.1; ISIZE is the input size modulo 2^32 + if (_value != (_inflater.getBytesWritten() & UINT_MAX)) throw new ZipException("Invalid input size"); // TODO ByteBuffer result = output == null ? BufferUtil.EMPTY_BUFFER : ByteBuffer.wrap(output); diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java index 93c7c4c3f37..64e113db32d 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/GZIPContentDecoderTest.java @@ -20,6 +20,8 @@ package org.eclipse.jetty.http; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStream; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.concurrent.atomic.AtomicInteger; @@ -27,10 +29,15 @@ import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; import org.eclipse.jetty.io.ArrayByteBufferPool; +import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -351,4 +358,97 @@ public class GZIPContentDecoderTest assertTrue(buffer.hasRemaining()); assertEquals(data2, StandardCharsets.UTF_8.decode(buffer).toString()); } + + // Signed Integer Max + final long INT_MAX = Integer.MAX_VALUE; + + // Unsigned Integer Max == 2^32 + final long UINT_MAX = 0xffffffffL; + + @ParameterizedTest + @ValueSource(longs = {INT_MAX, INT_MAX + 1, UINT_MAX, UINT_MAX + 1}) + public void testLargeGzipStream(long origSize) throws IOException + { + final int BUFSIZE = (1 * 1024 * 1024); // 1MB + + // Create a buffer to use over and over again to produce the uncompressed input + byte[] cbuf = "0123456789ABCDEFGHIJKLMOPQRSTUVWXYZ".getBytes(StandardCharsets.UTF_8); + byte[] buf = new byte[BUFSIZE]; + for (int off = 0; off < buf.length; ) + { + int len = Math.min(cbuf.length, buf.length - off); + System.arraycopy(cbuf, 0, buf, off, len); + off += len; + } + + GZIPContentDecoder decoder = new GZIPContentDecoder(BUFSIZE); + + try (GZIPDecoderOutputStream out = new GZIPDecoderOutputStream(decoder); + GZIPOutputStream outputStream = new GZIPOutputStream(out, BUFSIZE)) + { + long writtenBytes = 0L; + for (long bytesLeft = origSize; bytesLeft > 0; ) + { + int len = buf.length; + if (bytesLeft < buf.length) + { + len = (int)bytesLeft; + } + outputStream.write(buf, 0, len); + bytesLeft -= len; + writtenBytes += len; + } + outputStream.close(); + + assertThat("Written byte count", writtenBytes, is(origSize)); + assertThat("Decoded byte count", out.decodedByteCount, is(origSize)); + } + } + + public static class GZIPDecoderOutputStream extends OutputStream + { + private final GZIPContentDecoder decoder; + public long decodedByteCount = 0L; + + public GZIPDecoderOutputStream(GZIPContentDecoder decoder) + { + this.decoder = decoder; + } + + @Override + public void write(byte[] b, int off, int len) throws IOException + { + ByteBuffer buf = ByteBuffer.wrap(b, off, len); + decode(buf); + } + + @Override + public void write(byte[] b) throws IOException + { + ByteBuffer buf = ByteBuffer.wrap(b); + decode(buf); + } + + @Override + public void write(int b) throws IOException + { + ByteBuffer buf = BufferUtil.allocate(32); + buf.put((byte)(b & 0xFF)); + buf.flip(); + decode(buf); + } + + private void decode(ByteBuffer buffer) + { + while (buffer.hasRemaining()) + { + ByteBuffer decoded = decoder.decode(buffer); + if (decoded.hasRemaining()) + { + decodedByteCount += decoded.remaining(); + } + decoder.release(decoded); + } + } + } }