Add tests
Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/7467e596 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/7467e596 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/7467e596 Branch: refs/heads/master Commit: 7467e5966fea5cabfa47d189688a6b0bd88a7a6d Parents: 6457f26 Author: Roman Leventov <leventov...@gmail.com> Authored: Fri Nov 30 20:22:51 2018 +0100 Committer: Roman Leventov <leventov...@gmail.com> Committed: Fri Nov 30 20:22:51 2018 +0100 ---------------------------------------------------------------------- .../framework/imps/GzipCompressionProvider.java | 22 +++++++----- .../imps/GzipCompressionProviderTest.java | 37 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/7467e596/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java index 74d0347..fa357d2 100644 --- a/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java +++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/GzipCompressionProvider.java @@ -70,11 +70,14 @@ public class GzipCompressionProvider implements CompressionProvider /** 32-bit CRC and uncompressed data size */ private static final int GZIP_TRAILER_SIZE = Integer.BYTES + Integer.BYTES; + /** DEFLATE doesn't produce shorter compressed data */ + private static final int MIN_COMPRESSED_DATA_SIZE = 2; + /** * Since Deflaters and Inflaters are acquired and returned to the pools in try-finally blocks that are free of * blocking calls themselves, it's not expected that the number of objects in the pools could exceed the number of - * hardware threads on the machine much. Therefore it's accepted to have simple pools of strongly-referenced - * objects. + * hardware threads on the machine much. Therefore it's accepted to have simple "ever-growing" (in fact, no) pools + * of strongly-referenced objects. */ private static final ConcurrentLinkedQueue<Deflater> DEFLATER_POOL = new ConcurrentLinkedQueue<>(); private static final ConcurrentLinkedQueue<Inflater> INFLATER_POOL = new ConcurrentLinkedQueue<>(); @@ -135,8 +138,7 @@ public class GzipCompressionProvider implements CompressionProvider { break; } - // `+ 1` to ensure some growth on the sizes of 0 or 1 - int newResultLength = result.length + (result.length / 2) + 1; + int newResultLength = result.length + (result.length / 2); result = Arrays.copyOf(result, newResultLength); } // Write GZip trailer @@ -162,7 +164,7 @@ public class GzipCompressionProvider implements CompressionProvider if ( dataSize < 512 ) { // Assuming DEFLATE doesn't compress small data well - conservativeCompressedDataSizeEstimate = dataSize; + conservativeCompressedDataSizeEstimate = Math.max(dataSize, MIN_COMPRESSED_DATA_SIZE); } else { @@ -191,7 +193,7 @@ public class GzipCompressionProvider implements CompressionProvider ByteBuffer gzippedData = ByteBuffer.wrap(gzippedDataBytes); gzippedData.order(ByteOrder.LITTLE_ENDIAN); int headerSize = readGzipHeader(gzippedData); - if ( gzippedDataBytes.length < headerSize + GZIP_TRAILER_SIZE ) + if ( gzippedDataBytes.length < headerSize + MIN_COMPRESSED_DATA_SIZE + GZIP_TRAILER_SIZE ) { throw new EOFException("Too short GZipped data"); } @@ -220,7 +222,10 @@ public class GzipCompressionProvider implements CompressionProvider { break; } - else if ( inflater.needsInput() ) + // Just calling inflater.needsInput() doesn't work as expected, apparently it doesn't uphold it's own + // contract and could have needsInput() == true if numDecompressedBytes != 0 and that just means that + // there is not enough space in the result array + else if ( numDecompressedBytes == 0 && inflater.needsInput() ) { throw new ZipException("Corrupt GZipped data"); } @@ -231,9 +236,8 @@ public class GzipCompressionProvider implements CompressionProvider { throw new OutOfMemoryError("Unable to uncompress that much data into a single byte[] array"); } - // `+ 1` to ensure some growth on the sizes of 0 or 1 int newResultLength = - (int) Math.min((long) result.length + (result.length / 2) + 1, MAX_SAFE_JAVA_BYTE_ARRAY_SIZE); + (int) Math.min((long) result.length + (result.length / 2), MAX_SAFE_JAVA_BYTE_ARRAY_SIZE); if ( result.length != newResultLength ) { result = Arrays.copyOf(result, newResultLength); http://git-wip-us.apache.org/repos/asf/curator/blob/7467e596/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java ---------------------------------------------------------------------- diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java index 11b8c63..51edaba 100644 --- a/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java +++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/GzipCompressionProviderTest.java @@ -21,8 +21,11 @@ package org.apache.curator.framework.imps; import org.testng.Assert; import org.testng.annotations.Test; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Arrays; +import java.util.concurrent.ThreadLocalRandom; +import java.util.zip.GZIPOutputStream; public class GzipCompressionProviderTest { @@ -32,6 +35,8 @@ public class GzipCompressionProviderTest GzipCompressionProvider provider = new GzipCompressionProvider(); byte[] data = "Hello, world!".getBytes(); byte[] compressedData = provider.compress(null, data); + byte[] jdkCompressedData = jdkCompress(data); + Assert.assertTrue(Arrays.equals(compressedData, jdkCompressedData)); byte[] decompressedData = provider.decompress(null, compressedData); Assert.assertTrue(Arrays.equals(decompressedData, data)); } @@ -42,8 +47,10 @@ public class GzipCompressionProviderTest GzipCompressionProvider provider = new GzipCompressionProvider(); byte[] compressedData = provider.compress(null, new byte[0]); byte[] compressedData2 = GzipCompressionProvider.doCompress(new byte[0]); + byte[] jdkCompress = jdkCompress(new byte[0]); // Ensures GzipCompressionProvider.COMPRESSED_EMPTY_BYTES value is valid Assert.assertTrue(Arrays.equals(compressedData, compressedData2)); + Assert.assertTrue(Arrays.equals(compressedData, jdkCompress)); byte[] decompressedData = provider.decompress(null, compressedData); Assert.assertEquals(0, decompressedData.length); } @@ -85,4 +92,34 @@ public class GzipCompressionProviderTest } } } + + @Test + public void smokeTestRandomDataWithJdk() throws IOException + { + GzipCompressionProvider provider = new GzipCompressionProvider(); + ThreadLocalRandom random = ThreadLocalRandom.current(); + for (int len = 1; len < 100; len++) + { + byte[] data = new byte[len]; + for (int i = 0; i < 100; i++) { + byte[] compressedData = provider.compress(null, data); + byte[] jdkCompressedData = jdkCompress(data); + Assert.assertTrue(Arrays.equals(compressedData, jdkCompressedData)); + byte[] decompressedData = provider.decompress(null, compressedData); + Assert.assertTrue(Arrays.equals(decompressedData, data)); + // in the end of the iteration to test empty array first + random.nextBytes(data); + } + } + } + + private static byte[] jdkCompress(byte[] data) throws IOException + { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (GZIPOutputStream out = new GZIPOutputStream(bytes)) { + out.write(data); + out.finish(); + } + return bytes.toByteArray(); + } }