From 7a2a057c86e2b053e57f7d9ebdff9710eff3c1a4 Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Sun, 18 Oct 2020 22:24:16 +0000 Subject: [PATCH] #64817 - Fix issue in testXLSXinPPT git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1882623 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/extractor/ExtractorFactory.java | 14 ++++--- .../poi/sl/usermodel/SlideShowFactory.java | 5 ++- .../poi/ss/usermodel/WorkbookFactory.java | 12 +++--- .../xssf/usermodel/XSSFWorkbookFactory.java | 5 +-- .../poi/xssf/usermodel/TestXSSFBugs.java | 42 +++++++++++-------- 5 files changed, 43 insertions(+), 35 deletions(-) diff --git a/src/java/org/apache/poi/extractor/ExtractorFactory.java b/src/java/org/apache/poi/extractor/ExtractorFactory.java index 6b3ec00df6..a908a73085 100644 --- 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 83d3ae6e70..a69efaf8f8 100644 --- 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 ede792368a..c0f376f65f 100644 --- 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 725c941864..3ac689f372 100644 --- 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 4f7f1af19a..ffb67bf717 100644 --- 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()); } - */ } } } -- 2.39.5