#64817 - Fix issue in testXLSXinPPT

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

diff --git a/src/java/org/apache/poi/extractor/ExtractorFactory.java b/src/java/org/apache/poi/extractor/ExtractorFactory.java
index 6b3ec00df6813b6bb6c4390b640472c91bacbba4..a908a73085ad3a8853841bf06d71ee28a5e957aa 100644 (file)
--- a/src/java/org/apache/poi/extractor/ExtractorFactory.java
+++ b/src/java/org/apache/poi/extractor/ExtractorFactory.java
@@ -17,7 +17,7 @@
 package org.apache.poi.extractor;
 
 import static org.apache.poi.hssf.record.crypto.Biff8EncryptionKey.getCurrentUserPassword;
-import static org.apache.poi.poifs.crypt.EncryptionInfo.ENCRYPTION_INFO_ENTRY;
+import static org.apache.poi.poifs.crypt.Decryptor.DEFAULT_POIFS_ENTRY;
 
 import java.io.File;
 import java.io.IOException;
@@ -168,9 +168,10 @@ public final class ExtractorFactory {
         }
 
         POIFSFileSystem poifs = new POIFSFileSystem(is);
-        boolean isOOXML = poifs.getRoot().hasEntry(ENCRYPTION_INFO_ENTRY);
+        DirectoryNode root = poifs.getRoot();
+        boolean isOOXML = root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE);
 
-        return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(poifs.getRoot(), password));
+        return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(root, password));
     }
 
     public static POITextExtractor createExtractor(File file) throws IOException {
@@ -193,8 +194,9 @@ public final class ExtractorFactory {
 
         POIFSFileSystem poifs = new POIFSFileSystem(file, true);
         try {
-            boolean isOOXML = poifs.getRoot().hasEntry(ENCRYPTION_INFO_ENTRY);
-            return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(poifs.getRoot(), password));
+            DirectoryNode root = poifs.getRoot();
+            boolean isOOXML = root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE);
+            return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(root, password));
         } catch (IOException | RuntimeException e) {
             IOUtils.closeQuietly(poifs);
             throw e;
@@ -223,7 +225,7 @@ public final class ExtractorFactory {
 
     public static POITextExtractor createExtractor(final DirectoryNode root, String password) throws IOException {
         // Encrypted OOXML files go inside OLE2 containers, is this one?
-        if (root.hasEntry(Decryptor.DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE)) {
+        if (root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE)) {
             return wp(FileMagic.OOXML, w -> w.create(root, password));
         } else {
             return wp(FileMagic.OLE2, w ->  w.create(root, password));

diff --git a/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java b/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java
index 83d3ae6e70daefc97afbd61112e1013a79a73626..a69efaf8f8cd350d21dd693fbc9ce7b0a609c76c 100644 (file)
--- a/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java
+++ b/src/java/org/apache/poi/sl/usermodel/SlideShowFactory.java
@@ -17,7 +17,7 @@
 package org.apache.poi.sl.usermodel;
 
 import static org.apache.poi.extractor.ExtractorFactory.OOXML_PACKAGE;
-import static org.apache.poi.poifs.crypt.EncryptionInfo.ENCRYPTION_INFO_ENTRY;
+import static org.apache.poi.poifs.crypt.Decryptor.DEFAULT_POIFS_ENTRY;
 
 import java.io.BufferedInputStream;
 import java.io.File;
@@ -198,7 +198,8 @@ public final class SlideShowFactory {
         }
 
         POIFSFileSystem poifs = new POIFSFileSystem(is);
-        boolean isOOXML = poifs.getRoot().hasEntry(ENCRYPTION_INFO_ENTRY);
+        DirectoryNode root = poifs.getRoot();
+        boolean isOOXML = root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE);
 
         return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(poifs.getRoot(), password));
     }

diff --git a/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java b/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
index ede792368a1c0a8ba948a33f214bc7560d931709..c0f376f65f06e7147cdb31463cf9a28509b42652 100644 (file)
--- a/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
+++ b/src/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
@@ -17,7 +17,7 @@
 package org.apache.poi.ss.usermodel;
 
 import static org.apache.poi.extractor.ExtractorFactory.OOXML_PACKAGE;
-import static org.apache.poi.poifs.crypt.EncryptionInfo.ENCRYPTION_INFO_ENTRY;
+import static org.apache.poi.poifs.crypt.Decryptor.DEFAULT_POIFS_ENTRY;
 
 import java.io.BufferedInputStream;
 import java.io.File;
@@ -31,7 +31,6 @@ import java.util.ServiceLoader;
 import org.apache.poi.EmptyFileException;
 import org.apache.poi.EncryptedDocumentException;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
-import org.apache.poi.poifs.crypt.Decryptor;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.FileMagic;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
@@ -132,7 +131,7 @@ public final class WorkbookFactory {
      */
     public static Workbook create(final DirectoryNode root, String password) throws IOException {
         // Encrypted OOXML files go inside OLE2 containers, is this one?
-        if (root.hasEntry(Decryptor.DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE)) {
+        if (root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE)) {
             return wp(FileMagic.OOXML, w -> w.create(root, password));
         } else {
             return wp(FileMagic.OLE2, w ->  w.create(root, password));
@@ -205,9 +204,10 @@ public final class WorkbookFactory {
         }
 
         POIFSFileSystem poifs = new POIFSFileSystem(is);
-        boolean isOOXML = poifs.getRoot().hasEntry(ENCRYPTION_INFO_ENTRY);
+        DirectoryNode root = poifs.getRoot();
+        boolean isOOXML = root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE);
 
-        return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(poifs.getRoot(), password));
+        return wp(isOOXML ? FileMagic.OOXML : fm, w -> w.create(root, password));
     }
 
     /**
@@ -279,7 +279,7 @@ public final class WorkbookFactory {
             final boolean ooxmlEnc;
             try (POIFSFileSystem fs = new POIFSFileSystem(file, true)) {
                 DirectoryNode root = fs.getRoot();
-                ooxmlEnc = root.hasEntry(Decryptor.DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE);
+                ooxmlEnc = root.hasEntry(DEFAULT_POIFS_ENTRY) || root.hasEntry(OOXML_PACKAGE);
             }
             return wp(ooxmlEnc ? FileMagic.OOXML : fm, w -> w.create(file, password, readOnly));
         }

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java
index 725c9418640d628cf19ac05ee6f4cb3e4def957f..3ac689f372af389d212f986d30c59fed44647ba4 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbookFactory.java
@@ -55,10 +55,7 @@ public class XSSFWorkbookFactory implements WorkbookProvider {
     public XSSFWorkbook create(DirectoryNode root, String password) throws IOException {
         try (InputStream stream = DocumentFactoryHelper.getDecryptedStream(root, password)) {
             return create(stream);
-        } finally {
-            // as we processed the full stream already, we can close the filesystem here
-            // otherwise file handles are leaked
-            root.getFileSystem().close();
+            // don't close root.getFileSystem() otherwise the container filesystem becomes invalid
         }
     }
 

diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
index 4f7f1af19ab2773be4e033efac81fb95d43f4dea..ffb67bf717271c73f1b177d77ac25e0a5ec7481f 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
@@ -17,6 +17,7 @@
 
 package org.apache.poi.xssf.usermodel;
 
+import static org.apache.poi.extractor.ExtractorFactory.OOXML_PACKAGE;
 import static org.apache.poi.openxml4j.opc.TestContentType.isOldXercesActive;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
@@ -52,13 +53,10 @@ import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
 import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.common.usermodel.HyperlinkType;
-import org.apache.poi.hslf.usermodel.HSLFObjectData;
-import org.apache.poi.hslf.usermodel.HSLFSlideShow;
 import org.apache.poi.hssf.HSSFITestDataProvider;
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
-import org.apache.poi.hslf.HSLFTestDataSamples;
 import org.apache.poi.ooxml.POIXMLDocumentPart;
 import org.apache.poi.ooxml.POIXMLDocumentPart.RelationPart;
 import org.apache.poi.ooxml.POIXMLException;
@@ -73,9 +71,14 @@ import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.poi.openxml4j.opc.PackagingURIHelper;
 import org.apache.poi.openxml4j.util.ZipSecureFile;
+import org.apache.poi.poifs.filesystem.DirectoryNode;
 import org.apache.poi.poifs.filesystem.DocumentEntry;
 import org.apache.poi.poifs.filesystem.DocumentInputStream;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
+import org.apache.poi.sl.usermodel.ObjectShape;
+import org.apache.poi.sl.usermodel.Slide;
+import org.apache.poi.sl.usermodel.SlideShow;
+import org.apache.poi.sl.usermodel.SlideShowFactory;
 import org.apache.poi.ss.ITestDataProvider;
 import org.apache.poi.ss.SpreadsheetVersion;
 import org.apache.poi.ss.formula.ConditionalFormattingEvaluator;
@@ -3573,27 +3576,33 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
             int activeSheet = wb.getActiveSheetIndex();
         }
     }
-    
+
+
+
     @Test
     public void testXLSXinPPT() throws Exception {
-        try (HSLFSlideShow ppt = new HSLFSlideShow(
-                HSLFTestDataSamples.openSampleFileStream("testPPT_oleWorkbook.ppt"))) {
-            assertEquals(1, ppt.getEmbeddedObjects().length);
+        try (SlideShow<?,?> ppt = SlideShowFactory.create(
+                POIDataSamples.getSlideShowInstance().openResourceAsStream("testPPT_oleWorkbook.ppt"))) {
 
-            HSLFObjectData data = ppt.getEmbeddedObjects()[0];
-            assertEquals(null, data.getFileName());
+            Slide<?, ?> slide = ppt.getSlides().get(1);
+            ObjectShape<?,?> oleShape = (ObjectShape<?,?>)slide.getShapes().get(2);
+
+            org.apache.poi.sl.usermodel.ObjectData data = oleShape.getObjectData();
+            assertNull(data.getFileName());
 
             // Will be OOXML wrapped in OLE2, not directly SpreadSheet
             POIFSFileSystem fs = new POIFSFileSystem(data.getInputStream());
-            assertEquals(true, fs.getRoot().hasEntry("Package"));
-            assertEquals(false, fs.getRoot().hasEntry("Workbook"));
+            assertTrue(fs.getRoot().hasEntry(OOXML_PACKAGE));
+            assertFalse(fs.getRoot().hasEntry("Workbook"));
+
 
             // Can fetch Package to get OOXML
-            DocumentInputStream dis = new DocumentInputStream((DocumentEntry)fs.getRoot().getEntry("Package"));
-            try (OPCPackage pkg = OPCPackage.open(dis)) {
-                XSSFWorkbook wb = new XSSFWorkbook(pkg);
+            DirectoryNode root = fs.getRoot();
+            DocumentEntry docEntry = (DocumentEntry) root.getEntry(OOXML_PACKAGE);
+            try (DocumentInputStream dis = new DocumentInputStream(docEntry);
+                 OPCPackage pkg = OPCPackage.open(dis);
+                 XSSFWorkbook wb = new XSSFWorkbook(pkg)) {
                 assertEquals(1, wb.getNumberOfSheets());
-                wb.close();
             }
 
             // Via the XSSF Factory
@@ -3602,15 +3611,14 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
                 assertEquals(1, wb.getNumberOfSheets());
             }
 
+
             // Or can open via the normal Factory, as stream or OLE2
-            /* this part test is currently broken - https://bz.apache.org/bugzilla/show_bug.cgi?id=64817
             try (Workbook wb = WorkbookFactory.create(fs)) {
                 assertEquals(1, wb.getNumberOfSheets());
             }
             try (Workbook wb = WorkbookFactory.create(data.getInputStream())) {
                 assertEquals(1, wb.getNumberOfSheets());
             }
-             */
         }
     }
 }