You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2020/11/01 09:22:10 UTC

svn commit: r1883038 - in /poi/trunk/src: java/org/apache/poi/ java/org/apache/poi/poifs/nio/ ooxml/testcases/org/apache/poi/ss/tests/ ooxml/testcases/org/apache/poi/xssf/usermodel/ scratchpad/src/org/apache/poi/hslf/usermodel/ testcases/org/apache/poi...

Author: centic
Date: Sun Nov  1 09:22:09 2020
New Revision: 1883038

URL: http://svn.apache.org/viewvc?rev=1883038&view=rev
Log:
Fix file-handle-leaks when re-writing documents or slideshows

When replacing a directory, we should close the previous one
Close some resources in tests

Modified:
    poi/trunk/src/java/org/apache/poi/POIDocument.java
    poi/trunk/src/java/org/apache/poi/poifs/nio/ByteArrayBackedDataSource.java
    poi/trunk/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java
    poi/trunk/src/ooxml/testcases/org/apache/poi/ss/tests/TestWorkbookFactory.java
    poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidationConstraint.java
    poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java
    poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java
    poi/trunk/src/testcases/org/apache/poi/poifs/filesystem/TestFileSystemBugs.java

Modified: poi/trunk/src/java/org/apache/poi/POIDocument.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/POIDocument.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/POIDocument.java (original)
+++ poi/trunk/src/java/org/apache/poi/POIDocument.java Sun Nov  1 09:22:09 2020
@@ -463,7 +463,21 @@ public abstract class POIDocument implem
      * @param newDirectory the new directory
      */
     @Internal
-    protected void replaceDirectory(DirectoryNode newDirectory) {
+    protected void replaceDirectory(DirectoryNode newDirectory) throws IOException {
+        if (
+                // do not close if it is actually the same directory or
+                newDirectory == directory ||
+
+                // also for different directories, but same FileSystem
+                (newDirectory != null && directory != null && newDirectory.getFileSystem() == directory.getFileSystem())) {
+            return;
+        }
+
+        // close any previous opened DataSource
+        if (directory != null && directory.getFileSystem() != null) {
+            directory.getFileSystem().close();
+        }
+
         directory = newDirectory;
     }
 

Modified: poi/trunk/src/java/org/apache/poi/poifs/nio/ByteArrayBackedDataSource.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/ByteArrayBackedDataSource.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/poifs/nio/ByteArrayBackedDataSource.java (original)
+++ poi/trunk/src/java/org/apache/poi/poifs/nio/ByteArrayBackedDataSource.java Sun Nov  1 09:22:09 2020
@@ -32,15 +32,16 @@ public class ByteArrayBackedDataSource e
 
    private byte[] buffer;
    private long size;
-   
+
    public ByteArrayBackedDataSource(byte[] data, int size) { // NOSONAR
       this.buffer = data;
       this.size = size;
    }
+   
    public ByteArrayBackedDataSource(byte[] data) {
       this(data, data.length);
    }
-                
+
    @Override
    public ByteBuffer read(int length, long position) {
       if(position >= size) {
@@ -49,28 +50,28 @@ public class ByteArrayBackedDataSource e
                position + " in stream of length " + size
          );
       }
-      
+
       int toRead = (int)Math.min(length, size - position);
       return ByteBuffer.wrap(buffer, (int)position, toRead);
    }
-   
+
    @Override
    public void write(ByteBuffer src, long position) {
       // Extend if needed
-      long endPosition = position + src.capacity(); 
+      long endPosition = position + src.capacity();
       if(endPosition > buffer.length) {
          extend(endPosition);
       }
-      
+
       // Now copy
       src.get(buffer, (int)position, src.capacity());
-      
+
       // Update size if needed
       if(endPosition > size) {
          size = endPosition;
       }
    }
-   
+
    private void extend(long length) {
       // Consider extending by a bit more than requested
       long difference = length - buffer.length;
@@ -86,17 +87,17 @@ public class ByteArrayBackedDataSource e
       System.arraycopy(buffer, 0, nb, 0, (int)size);
       buffer = nb;
    }
-   
+
    @Override
    public void copyTo(OutputStream stream) throws IOException {
       stream.write(buffer, 0, (int)size);
    }
-   
+
    @Override
    public long size() {
       return size;
    }
-   
+
    @Override
    public void close() {
       buffer = null;

Modified: poi/trunk/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java (original)
+++ poi/trunk/src/java/org/apache/poi/poifs/nio/FileBackedDataSource.java Sun Nov  1 09:22:09 2020
@@ -43,7 +43,7 @@ public class FileBackedDataSource extend
 
     private final boolean writable;
     // remember file base, which needs to be closed too
-    private RandomAccessFile srcFile;
+    private final RandomAccessFile srcFile;
 
     // Buffers which map to a file-portion are not closed automatically when the Channel is closed
     // therefore we need to keep the list of mapped buffers and do some ugly reflection to try to
@@ -63,11 +63,15 @@ public class FileBackedDataSource extend
     }
 
     public FileBackedDataSource(RandomAccessFile srcFile, boolean readOnly) {
-        this(srcFile.getChannel(), readOnly);
-        this.srcFile = srcFile;
+        this(srcFile, srcFile.getChannel(), readOnly);
     }
 
     public FileBackedDataSource(FileChannel channel, boolean readOnly) {
+        this(null, channel, readOnly);
+    }
+
+    private FileBackedDataSource(RandomAccessFile srcFile, FileChannel channel, boolean readOnly) {
+        this.srcFile = srcFile;
         this.channel = channel;
         this.writable = !readOnly;
     }

Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/ss/tests/TestWorkbookFactory.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/ss/tests/TestWorkbookFactory.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/ss/tests/TestWorkbookFactory.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/ss/tests/TestWorkbookFactory.java Sun Nov  1 09:22:09 2020
@@ -346,6 +346,8 @@ public final class TestWorkbookFactory {
             fail("Shouldn't be able to open with the wrong password");
         } catch (EncryptedDocumentException e) {
             // expected here
+        } finally {
+            wb.close();
         }
 
         try {

Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidationConstraint.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidationConstraint.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidationConstraint.java (original)
+++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDataValidationConstraint.java Sun Nov  1 09:22:09 2020
@@ -26,8 +26,7 @@ import org.apache.poi.ss.usermodel.DataV
 import org.apache.poi.ss.util.CellReference;
 import org.junit.Test;
 
-import java.util.Collections;
-import java.util.stream.Collectors;
+import java.io.IOException;
 import java.util.stream.IntStream;
 
 public class TestXSSFDataValidationConstraint {
@@ -46,7 +45,7 @@ public class TestXSSFDataValidationConst
         // FIXME: whitespace wasn't stripped
         assertEquals(literal, constraint.getFormula1());
     }
-    
+
     @Test
     public void listLiteralsQuotesAreStripped_arrayConstructor() {
         // literal list, using array constructor
@@ -63,13 +62,14 @@ public class TestXSSFDataValidationConst
         String[] literal = IntStream.range(0, 129).mapToObj(i -> "a").toArray(String[]::new);
         assertThrows(IllegalArgumentException.class, () -> new XSSFDataValidationConstraint(literal));
     }
-    
+
     @Test
-    public void dataValidationListLiteralTooLongFromFile() {
-        XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("DataValidationListTooLong.xlsx");
-        XSSFFormulaEvaluator fEval = wb.getCreationHelper().createFormulaEvaluator();
-        DataValidationEvaluator dvEval = new DataValidationEvaluator(wb, fEval);
-        assertThrows(IllegalArgumentException.class, () -> dvEval.getValidationValuesForCell(new CellReference("Sheet0!A1")));
+    public void dataValidationListLiteralTooLongFromFile() throws IOException {
+        try (XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("DataValidationListTooLong.xlsx")) {
+            XSSFFormulaEvaluator fEval = wb.getCreationHelper().createFormulaEvaluator();
+            DataValidationEvaluator dvEval = new DataValidationEvaluator(wb, fEval);
+            assertThrows(IllegalArgumentException.class, () -> dvEval.getValidationValuesForCell(new CellReference("Sheet0!A1")));
+        }
     }
 
     @Test
@@ -80,7 +80,7 @@ public class TestXSSFDataValidationConst
         assertNull(constraint.getExplicitListValues());
         assertEquals("A1:A5", constraint.getFormula1());
     }
-    
+
     @Test
     public void namedRangeReference() {
         // named range list
@@ -90,4 +90,4 @@ public class TestXSSFDataValidationConst
         assertEquals("MyNamedRange", constraint.getFormula1());
     }
 
-}        
+}

Modified: poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java (original)
+++ poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShow.java Sun Nov  1 09:22:09 2020
@@ -1263,7 +1263,7 @@ public final class HSLFSlideShow extends
 	}
 
 	@Override
-	protected void replaceDirectory(DirectoryNode newDirectory) {
+	protected void replaceDirectory(DirectoryNode newDirectory) throws IOException {
 		getSlideShowImpl().replaceDirectoryImpl(newDirectory);
 	}
 

Modified: poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java (original)
+++ poi/trunk/src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFSlideShowImpl.java Sun Nov  1 09:22:09 2020
@@ -872,7 +872,7 @@ public final class HSLFSlideShowImpl ext
         return super.initDirectory();
     }
 
-    void replaceDirectoryImpl(DirectoryNode newDirectory) {
+    void replaceDirectoryImpl(DirectoryNode newDirectory) throws IOException {
         super.replaceDirectory(newDirectory);
     }
 

Modified: poi/trunk/src/testcases/org/apache/poi/poifs/filesystem/TestFileSystemBugs.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/poifs/filesystem/TestFileSystemBugs.java?rev=1883038&r1=1883037&r2=1883038&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/poifs/filesystem/TestFileSystemBugs.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/poifs/filesystem/TestFileSystemBugs.java Sun Nov  1 09:22:09 2020
@@ -37,8 +37,8 @@ import org.junit.Test;
  * Tests bugs for POIFSFileSystem
  */
 public final class TestFileSystemBugs {
-    private static POIDataSamples _samples = POIDataSamples.getPOIFSInstance();
-    private static POIDataSamples _ssSamples = POIDataSamples.getSpreadSheetInstance();
+    private static final POIDataSamples _samples = POIDataSamples.getPOIFSInstance();
+    private static final POIDataSamples _ssSamples = POIDataSamples.getSpreadSheetInstance();
 
     private List<POIFSFileSystem> openedFSs;
 
@@ -110,7 +110,7 @@ public final class TestFileSystemBugs {
         assertTrue(entry.isDocumentEntry());
         assertEquals("\u0001CompObj", entry.getName());
     }
-    
+
     /**
      * Ensure that a file with a corrupted property in the
      *  properties table can still be loaded, and the remaining
@@ -123,7 +123,7 @@ public final class TestFileSystemBugs {
         DirectoryNode root = openSample("unknown_properties.msg");
         assertEquals(42, root.getEntryCount());
     }
-    
+
     /**
      * With heavily nested documents, ensure we still re-write the same
      */



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org