commons-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bode...@apache.org
Subject [05/10] commons-compress git commit: COMPRESS-382 and COMPRESS-386 -- take 3, create static MemoryLimit and remove new ctors.
Date Tue, 25 Apr 2017 18:46:05 GMT
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 <tallison@mitre.org>
Authored: Mon Apr 24 21:06:35 2017 -0400
Committer: tballison <tallison@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 &lt; -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 &gt; {@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 {


Mime
View raw message