COMPRESS-382 and COMPRESS-386 -- take 3, create static MemoryLimit and remove new ctors.
Project: http://git-wip-us.apache.org/repos/asf/commons-compress/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-compress/commit/0e8ff9c4 Tree: http://git-wip-us.apache.org/repos/asf/commons-compress/tree/0e8ff9c4 Diff: http://git-wip-us.apache.org/repos/asf/commons-compress/diff/0e8ff9c4 Branch: refs/heads/master Commit: 0e8ff9c44058afbca9c9126a8feebe41cd682626 Parents: b10528a Author: tballison <talli...@mitre.org> Authored: Mon Apr 24 21:06:35 2017 -0400 Committer: tballison <talli...@mitre.org> Committed: Mon Apr 24 21:06:35 2017 -0400 ---------------------------------------------------------------------- .../apache/commons/compress/MemoryLimit.java | 65 ++++++++++++++++++++ .../compressors/CompressorStreamFactory.java | 36 ++--------- .../lzma/LZMACompressorInputStream.java | 19 +++--- .../compressors/xz/XZCompressorInputStream.java | 32 +--------- .../compressors/z/ZCompressorInputStream.java | 10 +-- .../commons/compress/MemoryLimitTest.java | 29 +++++++++ .../compressors/DetectCompressorTestCase.java | 21 ++++++- 7 files changed, 131 insertions(+), 81 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/main/java/org/apache/commons/compress/MemoryLimit.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/MemoryLimit.java b/src/main/java/org/apache/commons/compress/MemoryLimit.java new file mode 100644 index 0000000..e03e8e7 --- /dev/null +++ b/src/main/java/org/apache/commons/compress/MemoryLimit.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.commons.compress; + +/** + * + * During initialization, some streams compute expected memory use. + * They should check this value and throw a MemoryLimitException if the + * estimated memory use is greater that {@link MemoryLimit#MEMORY_LIMIT_IN_KB}. + * <p/> + * During compression/archiving, streams can allocate byte arrays based + * on a value read in from the stream. Corrupt files can cause compressors/archivers + * to cause {@link OutOfMemoryError}s. Compressors/archivers should check + * this maximum threshold before allocating memory and throw a {@link MemoryLimitException} + * if the allocation would exceed this limit. + * <p/> + * To avoid changes in legacy behavior, {@link MemoryLimit#MEMORY_LIMIT_IN_KB} + * is set to {@link MemoryLimit#NO_LIMIT}. However, in applications that might + * encounter untrusted/corrupt files, we encourage setting the limit to something + * reasonable for the application. + * <p/> + * As of 1.14, this limit should be observed when instantiating CompressorStreams. + * Work remains to propagate memory limit checks throughout the codebase. + * + * @since 1.14 + */ +public class MemoryLimit { + + public static final int NO_LIMIT = -1; + public static volatile int MEMORY_LIMIT_IN_KB = NO_LIMIT; + + /** + * Sets {@link MemoryLimit#MEMORY_LIMIT_IN_KB}. + * @param memoryLimitInKb limit in kilobytes + * + * @throws IllegalArgumentException if value is < -1 + */ + public static void setMemoryLimitInKb(int memoryLimitInKb) { + if (memoryLimitInKb < -1) { + throw new IllegalArgumentException("MemoryLimit must be > -2"); + } + //TODO: do we want to set an absolute upper limit?! + MEMORY_LIMIT_IN_KB = memoryLimitInKb; + } + + public static int getMemoryLimitInKb() { + return MEMORY_LIMIT_IN_KB; + } +} http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/main/java/org/apache/commons/compress/compressors/CompressorStreamFactory.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/CompressorStreamFactory.java b/src/main/java/org/apache/commons/compress/compressors/CompressorStreamFactory.java index f3433d9..7daa291 100644 --- a/src/main/java/org/apache/commons/compress/compressors/CompressorStreamFactory.java +++ b/src/main/java/org/apache/commons/compress/compressors/CompressorStreamFactory.java @@ -349,52 +349,28 @@ public class CompressorStreamFactory implements CompressorStreamProvider { */ private volatile boolean decompressConcatenated = false; - private final int memoryLimitInKb; /** * Create an instance with the decompress Concatenated option set to false. */ public CompressorStreamFactory() { this.decompressUntilEOF = null; - this.memoryLimitInKb = -1; } /** * Create an instance with the provided decompress Concatenated option. - * + * * @param decompressUntilEOF * if true, decompress until the end of the input; if false, stop * after the first stream and leave the input position to point * to the next byte after the stream. This setting applies to the * gzip, bzip2 and xz formats only. - * - * @param memoryLimitInKb - * Some streams require allocation of potentially significant - * byte arrays/tables, and they can offer checks to prevent OOMs - * on corrupt files. Set the maximum allowed memory allocation in KBs. - * - * @since 1.14 + * @since 1.10 */ - public CompressorStreamFactory(final boolean decompressUntilEOF, final int memoryLimitInKb) { + public CompressorStreamFactory(final boolean decompressUntilEOF) { this.decompressUntilEOF = Boolean.valueOf(decompressUntilEOF); // Also copy to existing variable so can continue to use that as the // current value this.decompressConcatenated = decompressUntilEOF; - this.memoryLimitInKb = memoryLimitInKb; - } - - - /** - * Create an instance with the provided decompress Concatenated option. - * - * @param decompressUntilEOF - * if true, decompress until the end of the input; if false, stop - * after the first stream and leave the input position to point - * to the next byte after the stream. This setting applies to the - * gzip, bzip2 and xz formats only. - * @since 1.10 - */ - public CompressorStreamFactory(final boolean decompressUntilEOF) { - this(decompressUntilEOF, -1); } /** @@ -529,14 +505,14 @@ public class CompressorStreamFactory implements CompressorStreamProvider { if (!XZUtils.isXZCompressionAvailable()) { throw new CompressorException("XZ compression is not available."); } - return new XZCompressorInputStream(in, actualDecompressConcatenated, memoryLimitInKb); + return new XZCompressorInputStream(in, actualDecompressConcatenated); } if (LZMA.equalsIgnoreCase(name)) { if (!LZMAUtils.isLZMACompressionAvailable()) { throw new CompressorException("LZMA compression is not available"); } - return new LZMACompressorInputStream(in, memoryLimitInKb); + return new LZMACompressorInputStream(in); } if (PACK200.equalsIgnoreCase(name)) { @@ -552,7 +528,7 @@ public class CompressorStreamFactory implements CompressorStreamProvider { } if (Z.equalsIgnoreCase(name)) { - return new ZCompressorInputStream(in, memoryLimitInKb); + return new ZCompressorInputStream(in); } if (DEFLATE.equalsIgnoreCase(name)) { http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/main/java/org/apache/commons/compress/compressors/lzma/LZMACompressorInputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/lzma/LZMACompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/lzma/LZMACompressorInputStream.java index 7782be8..6931541 100644 --- a/src/main/java/org/apache/commons/compress/compressors/lzma/LZMACompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/lzma/LZMACompressorInputStream.java @@ -21,6 +21,7 @@ package org.apache.commons.compress.compressors.lzma; import java.io.IOException; import java.io.InputStream; +import org.apache.commons.compress.MemoryLimit; import org.apache.commons.compress.MemoryLimitException; import org.tukaani.xz.LZMAInputStream; @@ -33,32 +34,26 @@ import org.apache.commons.compress.compressors.CompressorInputStream; public class LZMACompressorInputStream extends CompressorInputStream { private final InputStream in; - public LZMACompressorInputStream(final InputStream inputStream) - throws IOException { - in = new LZMAInputStream(inputStream, -1); - } - /** * Creates a new input stream that decompresses LZMA-compressed data * from the specified input stream. * * @param inputStream where to read the compressed data * - * @param memoryLimitInKb calculated memory use threshold. Throws MemoryLimitException - * if calculate memory use is above this threshold - * * @throws IOException if the input is not in the .lzma format, * the input is corrupt or truncated, the .lzma * headers specify sizes that are not supported - * by this implementation, or the underlying - * <code>inputStream</code> throws an exception + * by this implementation, the underlying + * <code>inputStream</code> throws an exception or + * if the calculated memory usage + * is > {@link MemoryLimit#MEMORY_LIMIT_IN_KB}. * * @since 1.14 */ - public LZMACompressorInputStream(final InputStream inputStream, int memoryLimitInKb) + public LZMACompressorInputStream(final InputStream inputStream) throws IOException { try { - in = new LZMAInputStream(inputStream, memoryLimitInKb); + in = new LZMAInputStream(inputStream, MemoryLimit.getMemoryLimitInKb()); } catch (org.tukaani.xz.MemoryLimitException e) { //convert to commons-compress exception throw new MemoryLimitException("exceeded calculated memory limit", e); http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/main/java/org/apache/commons/compress/compressors/xz/XZCompressorInputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/xz/XZCompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/xz/XZCompressorInputStream.java index b378212..3f977ef 100644 --- a/src/main/java/org/apache/commons/compress/compressors/xz/XZCompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/xz/XZCompressorInputStream.java @@ -21,6 +21,7 @@ package org.apache.commons.compress.compressors.xz; import java.io.IOException; import java.io.InputStream; +import org.apache.commons.compress.MemoryLimit; import org.apache.commons.compress.MemoryLimitException; import org.tukaani.xz.XZ; import org.tukaani.xz.SingleXZInputStream; @@ -94,38 +95,11 @@ public class XZCompressorInputStream extends CompressorInputStream { public XZCompressorInputStream(final InputStream inputStream, final boolean decompressConcatenated) throws IOException { - this(inputStream, decompressConcatenated, -1); - } - /** - * Creates a new input stream that decompresses XZ-compressed data - * from the specified input stream. - * - * @param inputStream where to read the compressed data - * @param decompressConcatenated - * if true, decompress until the end of the - * input; if false, stop after the first .xz - * stream and leave the input position to point - * to the next byte after the .xz stream - * @param memoryLimitInKb memory limit used when reading blocks. If - * the estimated memory limit is exceeded on {@link #read()}, - * a {@link MemoryLimitException} is thrown. - * - * @throws IOException if the input is not in the .xz format, - * the input is corrupt or truncated, the .xz - * headers specify options that are not supported - * by this implementation, - * or the underlying <code>inputStream</code> throws an exception - * - * @since 1.14 - */ - public XZCompressorInputStream(InputStream inputStream, - boolean decompressConcatenated, int memoryLimitInKb) - throws IOException { if (decompressConcatenated) { - in = new XZInputStream(inputStream, memoryLimitInKb); + in = new XZInputStream(inputStream, MemoryLimit.getMemoryLimitInKb()); } else { - in = new SingleXZInputStream(inputStream, memoryLimitInKb); + in = new SingleXZInputStream(inputStream, MemoryLimit.getMemoryLimitInKb()); } } http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java index 64387e3..ca61cd3 100644 --- a/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java +++ b/src/main/java/org/apache/commons/compress/compressors/z/ZCompressorInputStream.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.ByteOrder; +import org.apache.commons.compress.MemoryLimit; import org.apache.commons.compress.compressors.lzw.LZWInputStream; /** @@ -38,8 +39,7 @@ public class ZCompressorInputStream extends LZWInputStream { private final int maxCodeSize; private long totalCodesRead = 0; - public ZCompressorInputStream(final InputStream inputStream, int memoryLimitInKb) - throws IOException { + public ZCompressorInputStream(final InputStream inputStream) throws IOException { super(inputStream, ByteOrder.LITTLE_ENDIAN); final int firstByte = (int) in.readBits(8); final int secondByte = (int) in.readBits(8); @@ -52,13 +52,9 @@ public class ZCompressorInputStream extends LZWInputStream { if (blockMode) { setClearCode(DEFAULT_CODE_SIZE); } - initializeTables(maxCodeSize, memoryLimitInKb); + initializeTables(maxCodeSize, MemoryLimit.getMemoryLimitInKb()); clearEntries(); } - - public ZCompressorInputStream(final InputStream inputStream) throws IOException { - this(inputStream, -1); - } private void clearEntries() { setTableSize((1 << 8) + (blockMode ? 1 : 0)); http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/test/java/org/apache/commons/compress/MemoryLimitTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/compress/MemoryLimitTest.java b/src/test/java/org/apache/commons/compress/MemoryLimitTest.java new file mode 100644 index 0000000..fdbaa86 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/MemoryLimitTest.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.commons.compress; + +import org.junit.Test; + +public class MemoryLimitTest { + + @Test(expected = IllegalArgumentException.class) + public void testRangeCheck() throws Exception { + MemoryLimit.setMemoryLimitInKb(-2); + } +} http://git-wip-us.apache.org/repos/asf/commons-compress/blob/0e8ff9c4/src/test/java/org/apache/commons/compress/compressors/DetectCompressorTestCase.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/compress/compressors/DetectCompressorTestCase.java b/src/test/java/org/apache/commons/compress/compressors/DetectCompressorTestCase.java index b70d3c7..6c8543f 100644 --- a/src/test/java/org/apache/commons/compress/compressors/DetectCompressorTestCase.java +++ b/src/test/java/org/apache/commons/compress/compressors/DetectCompressorTestCase.java @@ -31,6 +31,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import org.apache.commons.compress.MemoryLimit; import org.apache.commons.compress.MemoryLimitException; import org.apache.commons.compress.MockEvilInputStream; import org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream; @@ -38,6 +39,8 @@ import org.apache.commons.compress.compressors.deflate.DeflateCompressorInputStr import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream; import org.apache.commons.compress.compressors.pack200.Pack200CompressorInputStream; import org.apache.commons.compress.compressors.xz.XZCompressorInputStream; +import org.junit.AfterClass; +import org.junit.Before; import org.junit.Test; @SuppressWarnings("deprecation") // deliberately tests setDecompressConcatenated @@ -71,6 +74,18 @@ public final class DetectCompressorTestCase { } } + @Before + public void setUp() { + //make sure to reset this before each test + MemoryLimit.setMemoryLimitInKb(MemoryLimit.NO_LIMIT); + } + + @AfterClass + public static void tearDown() { + //make sure this is really, truly reset after all the tests + MemoryLimit.setMemoryLimitInKb(MemoryLimit.NO_LIMIT); + } + private final TestData[] tests = { new TestData("multiple.bz2", new char[]{'a','b'}, factoryTrue, true), new TestData("multiple.bz2", new char[]{'a','b'}, factorySetTrue, true), @@ -190,7 +205,7 @@ public final class DetectCompressorTestCase { //This test is here instead of the xz unit test to make sure //that the parameter is properly passed via the CompressorStreamFactory try (InputStream compressorIs = getStreamFor("bla.tar.xz", 100)) { - int c = compressorIs.read(); + compressorIs.read(); } } @@ -202,8 +217,8 @@ public final class DetectCompressorTestCase { } private InputStream getStreamFor(final String fileName, final int memoryLimitInKb) throws Exception { - CompressorStreamFactory fac = new CompressorStreamFactory(true, - memoryLimitInKb); + MemoryLimit.setMemoryLimitInKb(memoryLimitInKb); + CompressorStreamFactory fac = new CompressorStreamFactory(true); InputStream is = new BufferedInputStream( new FileInputStream(getFile(fileName))); try {
