Bug 58499: Don't report Zip-Bomb for small files which should not cause memory issues anyway

Also make error message a bit more specific and list classname in Zip-Bomb-Error to make it easier for users what the
problem is and how to find out where the static methods are

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1709180 13f79535-47bb-0310-9956-ffa450edef68

src/ooxml/java/org/apache/poi/openxml4j/util/ZipSecureFile.java

@@ -47,6 +47,9 @@ public class ZipSecureFile extends ZipFile {
     private static double MIN_INFLATE_RATIO = 0.01d;
     private static long MAX_ENTRY_SIZE = 0xFFFFFFFFl;
     
+    // don't alert for expanded sizes smaller than 100k
+    private static long GRACE_ENTRY_SIZE = 100*1024;
+
     /**
      * Sets the ratio between de- and inflated bytes to detect zipbomb.
      * It defaults to 1% (= 0.01d), i.e. when the compression is better than
@@ -183,16 +186,37 @@ public class ZipSecureFile extends ZipFile {
 
         public void advance(int advance) throws IOException {
             counter += advance;
+            
             // check the file size first, in case we are working on uncompressed streams
-            if (counter < MAX_ENTRY_SIZE) {
+            if(counter > MAX_ENTRY_SIZE) {
-                if (cis == null) return;
+                throw new IOException("Zip bomb detected! The file would exceed the max size of the expanded data in the zip-file. "
-                double ratio = (double)cis.counter/(double)counter;
+                        + "This may indicates that the file is used to inflate memory usage and thus could pose a security risk. "
-                if (ratio >= MIN_INFLATE_RATIO) return;
+                        + "You can adjust this limit via ZipSecureFile.setMaxEntrySize() if you need to work with files which are very large. "
+                        + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter)
+                        + "Limits: MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE);
             }
-            throw new IOException("Zip bomb detected! The file would exceed certain limits which usually indicate that the file is used to inflate memory usage and thus could pose a security risk. "
+
-                    + "You can adjust these limits via setMinInflateRatio() and setMaxEntrySize() if you need to work with files which exceed these limits. "
+            // no expanded size?
+            if (cis == null) {
+                return;
+            }
+            
+            // don't alert for small expanded size
+            if (counter <= GRACE_ENTRY_SIZE) {
+                return;
+            }
+
+            double ratio = (double)cis.counter/(double)counter;
+            if (ratio >= MIN_INFLATE_RATIO) {
+                return;
+            }
+
+            // one of the limits was reached, report it
+            throw new IOException("Zip bomb detected! The file would exceed the max. ratio of compressed file size to the size of the expanded data. "
+                    + "This may indicate that the file is used to inflate memory usage and thus could pose a security risk. "
+                    + "You can adjust this limit via ZipSecureFile.setMinInflateRatio() if you need to work with files which exceed this limit. "
                     + "Counter: " + counter + ", cis.counter: " + (cis == null ? 0 : cis.counter) + ", ratio: " + (cis == null ? 0 : ((double)cis.counter)/counter)
-                    + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO + ", MAX_ENTRY_SIZE: " + MAX_ENTRY_SIZE);
+                    + "Limits: MIN_INFLATE_RATIO: " + MIN_INFLATE_RATIO);
         }
 
         public ZipEntry getNextEntry() throws IOException {

src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java

@@ -28,7 +28,6 @@ import static org.junit.Assert.fail;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.OutputStream;
 import java.lang.reflect.Field;
 
 import org.apache.poi.ss.usermodel.BaseTestWorkbook;
@@ -209,7 +208,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
     }
 
     @Test
-    public void sheetdataWriter(){
+    public void sheetdataWriter() throws IOException{
         SXSSFWorkbook wb = new SXSSFWorkbook();
         SXSSFSheet sh = wb.createSheet();
         SheetDataWriter wr = sh.getSheetDataWriter();
@@ -218,6 +217,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
         assertTrue(tmp.getName().startsWith("poi-sxssf-sheet"));
         assertTrue(tmp.getName().endsWith(".xml"));
         assertTrue(wb.dispose());
+        wb.close();
 
         wb = new SXSSFWorkbook();
         wb.setCompressTempFiles(true);
@@ -228,6 +228,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
         assertTrue(tmp.getName().startsWith("poi-sxssf-sheet-xml"));
         assertTrue(tmp.getName().endsWith(".gz"));
         assertTrue(wb.dispose());
+        wb.close();
 
         //Test escaping of Unicode control characters
         wb = new SXSSFWorkbook();
@@ -237,6 +238,7 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
         assertEquals("value?", cell.getStringCellValue());
 
         assertTrue(wb.dispose());
+        wb.close();
 
     }
 
@@ -399,10 +401,4 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook {
             }
         }
     }
-
-	private static class NullOutputStream extends OutputStream {
-		@Override
-		public void write(int b) throws IOException {
-		}
-	}
 }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java

@@ -81,19 +81,6 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         return wb.getWorkbook();
     }
 
-    @Test
-    public void windowOneDefaults() throws IOException {
-        HSSFWorkbook b = new HSSFWorkbook( );
-        try {
-            assertEquals(b.getActiveSheetIndex(), 0);
-            assertEquals(b.getFirstVisibleTab(), 0);
-        } catch (NullPointerException npe) {
-            fail("WindowOneRecord in Workbook is probably not initialized");
-        }
-        
-        b.close();
-    }
-
     /**
      * Tests for {@link HSSFWorkbook#isHidden()} etc
      * @throws IOException 

src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java

@@ -26,15 +26,16 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.ConcurrentModificationException;
 import java.util.Iterator;
 
-import junit.framework.AssertionFailedError;
-
 import org.apache.poi.ss.ITestDataProvider;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.junit.Test;
 
+import junit.framework.AssertionFailedError;
+
 /**
  * @author Yegor Kozlov
  */
@@ -753,4 +754,48 @@ public abstract class BaseTestWorkbook {
 					sheets[i], wb.getSheetAt(i).getSheetName());
 		}
 	}
+
+    protected static class NullOutputStream extends OutputStream {
+        public NullOutputStream() {
+        }
+
+        @Override
+        public void write(int b) throws IOException {
+        }
+    }
+
+    @Test
+    public void test58499() throws IOException {
+        Workbook workbook = _testDataProvider.createWorkbook();
+        Sheet sheet = workbook.createSheet();
+        for (int i = 0; i < 900; i++) {
+            Row r = sheet.createRow(i);
+            Cell c = r.createCell(0);
+            CellStyle cs = workbook.createCellStyle();
+            c.setCellStyle(cs);
+            c.setCellValue("AAA");                
+        }
+        OutputStream os = new NullOutputStream();
+        try {
+            workbook.write(os);
+        } finally {
+            os.close();
+        }
+        //workbook.dispose();
+        workbook.close();
+    }
+
+
+    @Test
+    public void windowOneDefaults() throws IOException {
+        Workbook b = _testDataProvider.createWorkbook();
+        try {
+            assertEquals(b.getActiveSheetIndex(), 0);
+            assertEquals(b.getFirstVisibleTab(), 0);
+        } catch (NullPointerException npe) {
+            fail("WindowOneRecord in Workbook is probably not initialized");
+        }
+        
+        b.close();
+    }
 }