From 01ea839250bfa8f8601a42dbece7d42d94f400f3 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 12:54:19 +0100 Subject: [PATCH 1/9] `open` methods should use checked exceptions Update StreamingWorkbookReader.java fix compile issue Update OoxmlReader.java --- .../pjfanning/xlsx/StreamingReader.java | 12 ++-- .../xlsx/exceptions/CheckedReadException.java | 20 +++++++ .../exceptions/ExcelCheckedException.java | 23 ++++++++ .../xlsx/impl/StreamingWorkbook.java | 13 +---- .../xlsx/impl/StreamingWorkbookReader.java | 56 +++++++------------ .../xlsx/impl/ooxml/OoxmlReader.java | 2 +- .../pjfanning/xlsx/ExpectedException.java | 8 +++ .../pjfanning/xlsx/StreamingSheetTest.java | 13 ++--- .../pjfanning/xlsx/StreamingWorkbookTest.java | 49 ++++++++-------- .../com/github/pjfanning/xlsx/TestUtils.java | 3 +- 10 files changed, 117 insertions(+), 82 deletions(-) create mode 100644 src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java create mode 100644 src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java create mode 100644 src/test/java/com/github/pjfanning/xlsx/ExpectedException.java diff --git a/src/main/java/com/github/pjfanning/xlsx/StreamingReader.java b/src/main/java/com/github/pjfanning/xlsx/StreamingReader.java index 526c22f6..5a3e10e3 100644 --- a/src/main/java/com/github/pjfanning/xlsx/StreamingReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/StreamingReader.java @@ -1,5 +1,6 @@ package com.github.pjfanning.xlsx; +import com.github.pjfanning.xlsx.exceptions.CheckedReadException; import com.github.pjfanning.xlsx.exceptions.OpenException; import com.github.pjfanning.xlsx.exceptions.ParseException; import com.github.pjfanning.xlsx.exceptions.ReadException; @@ -545,9 +546,10 @@ public Builder setFullFormatRichText(boolean fullFormatRichText) { * * @param is input stream to read in * @return A {@link Workbook} that can be read from - * @throws com.github.pjfanning.xlsx.exceptions.ReadException if there is an issue reading the stream + * @throws IOException if an error occurs while opening the file + * @throws CheckedReadException if an error occurs while reading the file */ - public Workbook open(InputStream is) throws OpenException, ReadException, ParseException { + public Workbook open(InputStream is) throws IOException, CheckedReadException { StreamingWorkbookReader workbookReader = new StreamingWorkbookReader(this); workbookReader.init(is); return new StreamingWorkbook(workbookReader); @@ -559,10 +561,10 @@ public Workbook open(InputStream is) throws OpenException, ReadException, ParseE * * @param file file to read in * @return built streaming reader instance - * @throws com.github.pjfanning.xlsx.exceptions.OpenException if there is an issue opening the file - * @throws com.github.pjfanning.xlsx.exceptions.ReadException if there is an issue reading the file + * @throws IOException if an error occurs while opening the file + * @throws CheckedReadException if an error occurs while reading the file */ - public Workbook open(File file) throws OpenException, ReadException, ParseException { + public Workbook open(File file) throws IOException, CheckedReadException { StreamingWorkbookReader workbookReader = new StreamingWorkbookReader(this); workbookReader.init(file); return new StreamingWorkbook(workbookReader); diff --git a/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java b/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java new file mode 100644 index 00000000..f4e9fb7a --- /dev/null +++ b/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java @@ -0,0 +1,20 @@ +package com.github.pjfanning.xlsx.exceptions; + +public class CheckedReadException extends ExcelCheckedException { + + public CheckedReadException() { + super(); + } + + public CheckedReadException(String msg) { + super(msg); + } + + public CheckedReadException(Exception e) { + super(e); + } + + public CheckedReadException(String msg, Exception e) { + super(msg, e); + } +} diff --git a/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java b/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java new file mode 100644 index 00000000..22cca68c --- /dev/null +++ b/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java @@ -0,0 +1,23 @@ +package com.github.pjfanning.xlsx.exceptions; + +/** + * A parent class for all the excel-streaming-reader specific Checked Exceptions (i.e. not Runtime Exceptions). + */ +public class ExcelCheckedException extends Exception { + + protected ExcelCheckedException() { + super(); + } + + protected ExcelCheckedException(String msg) { + super(msg); + } + + protected ExcelCheckedException(Exception e) { + super(e); + } + + protected ExcelCheckedException(String msg, Exception e) { + super(msg, e); + } +} diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbook.java b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbook.java index 13ee5cf7..2795901c 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbook.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbook.java @@ -108,12 +108,8 @@ public int getNumberOfSheets() { * {@inheritDoc} */ @Override - public Sheet getSheetAt(final int index) throws ReadException { - try { - return reader.getSheetAt(index); - } catch (XMLStreamException|IOException e) { - throw new ReadException(e); - } + public Sheet getSheetAt(final int index) { + return reader.getSheetAt(index); } /** @@ -122,14 +118,11 @@ public Sheet getSheetAt(final int index) throws ReadException { * @param name - of the sheet * @return Sheet with the name provided * @throws MissingSheetException if no sheet is found with the provided name - * @throws ReadException if there is a problem reading the Excel file */ @Override - public Sheet getSheet(String name) throws MissingSheetException, ReadException { + public Sheet getSheet(String name) throws MissingSheetException { try { return reader.getSheet(name); - } catch (XMLStreamException|IOException e) { - throw new ReadException(e); } catch (NoSuchElementException nse) { throw new MissingSheetException("Failed to find sheet: " + name); } diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java index bdef91e8..057156e0 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java @@ -4,11 +4,9 @@ import com.github.pjfanning.poi.xssf.streaming.TempFileSharedStringsTable; import com.github.pjfanning.xlsx.SharedStringsImplementationType; import com.github.pjfanning.xlsx.StreamingReader.Builder; -import com.github.pjfanning.xlsx.exceptions.ExcelRuntimeException; import com.github.pjfanning.xlsx.exceptions.MissingSheetException; +import com.github.pjfanning.xlsx.exceptions.CheckedReadException; import com.github.pjfanning.xlsx.exceptions.NotSupportedException; -import com.github.pjfanning.xlsx.exceptions.OpenException; -import com.github.pjfanning.xlsx.exceptions.ParseException; import com.github.pjfanning.xlsx.exceptions.ReadException; import com.github.pjfanning.xlsx.impl.ooxml.OoxmlStrictHelper; import com.github.pjfanning.xlsx.impl.ooxml.OoxmlReader; @@ -70,11 +68,10 @@ public StreamingWorkbookReader(Builder builder) { /** * Initializes the reader with the given input stream. * @param is the input stream to read from - * @throws OpenException if an error occurs while opening the file - * @throws ReadException if an error occurs while reading the file - * @throws ParseException if an error occurs while parsing the file + * @throws IOException if an error occurs while opening the file + * @throws CheckedReadException if an error occurs while reading the file */ - public void init(InputStream is) throws OpenException, ReadException, ParseException { + public void init(InputStream is) throws IOException, CheckedReadException { if (builder.avoidTempFiles()) { try { if(builder.getPassword() != null) { @@ -86,10 +83,10 @@ public void init(InputStream is) throws OpenException, ReadException, ParseExcep loadPackage(pkg); } catch(SAXException e) { IOUtils.closeQuietly(pkg); - throw new ParseException("Failed to parse stream", e); + throw new CheckedReadException("Failed to parse stream", e); } catch(IOException e) { IOUtils.closeQuietly(pkg); - throw new OpenException("Failed to open stream", e); + throw e; } catch(GeneralSecurityException e) { IOUtils.closeQuietly(pkg); throw new ReadException("Unable to read workbook - Decryption failed", e); @@ -106,7 +103,7 @@ public void init(InputStream is) throws OpenException, ReadException, ParseExcep } init(f); tmp = f; - } catch(OpenException | ReadException e) { + } catch(IOException | CheckedReadException e) { if (f != null && !f.delete()) { log.debug("failed to delete temp file"); } @@ -115,12 +112,12 @@ public void init(InputStream is) throws OpenException, ReadException, ParseExcep if (f != null && !f.delete()) { log.debug("failed to delete temp file"); } - throw new ReadException("Unsupported File Format (only xlsx files are supported)", e); - } catch(IOException | RuntimeException e) { + throw new CheckedReadException("Unsupported File Format (only xlsx files are supported)", e); + } catch(RuntimeException e) { if (f != null && !f.delete()) { log.debug("failed to delete temp file"); } - throw new ReadException("Unable to read input stream", e); + throw new CheckedReadException("Unable to read workbook", e); } } } @@ -128,11 +125,10 @@ public void init(InputStream is) throws OpenException, ReadException, ParseExcep /** * Initializes the reader with the given input stream. * @param f the file to read from - * @throws OpenException if an error occurs while opening the file - * @throws ReadException if an error occurs while reading the file - * @throws ParseException if an error occurs while parsing the file + * @throws IOException if an error occurs while opening the file + * @throws CheckedReadException if an error occurs while reading the file */ - public void init(File f) throws OpenException, ReadException, ParseException { + public void init(File f) throws IOException, CheckedReadException { try { if(builder.getPassword() != null) { POIFSFileSystem poifs = new POIFSFileSystem(f); @@ -143,25 +139,19 @@ public void init(File f) throws OpenException, ReadException, ParseException { loadPackage(pkg); } catch(SAXException e) { IOUtils.closeQuietly(pkg); - throw new ParseException("Failed to parse file", e); + throw new CheckedReadException("Failed to parse file", e); } catch(IOException e) { IOUtils.closeQuietly(pkg); - throw new OpenException("Failed to open file", e); + throw e; } catch(UnsupportedFileFormatException e) { IOUtils.closeQuietly(pkg); - throw new ReadException("Unsupported File Format (only xlsx files are supported)", e); - } catch(OpenXML4JException | XMLStreamException e) { - IOUtils.closeQuietly(pkg); - throw new ReadException("Unable to read workbook", e); + throw new CheckedReadException("Unsupported File Format (only xlsx files are supported)", e); } catch(GeneralSecurityException e) { IOUtils.closeQuietly(pkg); - throw new ReadException("Unable to read workbook - Decryption failed", e); - } catch(ExcelRuntimeException e) { - IOUtils.closeQuietly(pkg); - throw e; - } catch(RuntimeException e) { + throw new CheckedReadException("Unable to read workbook - Decryption failed", e); + } catch(OpenXML4JException | XMLStreamException | RuntimeException e) { IOUtils.closeQuietly(pkg); - throw new ReadException("Unable to read workbook", e); + throw new CheckedReadException("Unable to read workbook", e); } } @@ -240,10 +230,8 @@ private List loadSheets() { * @param idx index (0 based) * @return the sheet at the given index * @throws MissingSheetException if a sheet at the given index does not exist - * @throws IOException should never be thrown - * @throws XMLStreamException should never be thrown */ - public StreamingSheet getSheetAt(final int idx) throws MissingSheetException, IOException, XMLStreamException { + public StreamingSheet getSheetAt(final int idx) throws MissingSheetException { if (sheets != null && sheets.size() > idx) { return sheets.get(idx); } else { @@ -261,10 +249,8 @@ public StreamingSheet getSheetAt(final int idx) throws MissingSheetException, IO * @param name the name of the sheet to return * @return the sheet with the given name * @throws MissingSheetException if a sheet with the given name does not exist - * @throws IOException should never be thrown - * @throws XMLStreamException should never be thrown */ - public StreamingSheet getSheet(final String name) throws MissingSheetException, IOException, XMLStreamException { + public StreamingSheet getSheet(final String name) throws MissingSheetException { final int idx = ooxmlReader.getSheetIndex(name); return getSheetAt(idx); } diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java index 38f19e01..153b78c4 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java @@ -150,7 +150,7 @@ public SharedStrings getSharedStrings(StreamingReader.Builder builder) throws IO * @throws IOException if an error occurs while reading the styles. */ @Override - public StylesTable getStylesTable() throws IOException, InvalidFormatException { + public StylesTable getStylesTable() throws IOException { ArrayList parts = pkg.getPartsByContentType(XSSFRelation.STYLES.getContentType()); if (parts.isEmpty()) return null; diff --git a/src/test/java/com/github/pjfanning/xlsx/ExpectedException.java b/src/test/java/com/github/pjfanning/xlsx/ExpectedException.java new file mode 100644 index 00000000..1db373c4 --- /dev/null +++ b/src/test/java/com/github/pjfanning/xlsx/ExpectedException.java @@ -0,0 +1,8 @@ +package com.github.pjfanning.xlsx; + +final class ExpectedException extends RuntimeException { + + ExpectedException(Exception e) { + super(e); + } +} diff --git a/src/test/java/com/github/pjfanning/xlsx/StreamingSheetTest.java b/src/test/java/com/github/pjfanning/xlsx/StreamingSheetTest.java index b24d483e..ad2b6baf 100644 --- a/src/test/java/com/github/pjfanning/xlsx/StreamingSheetTest.java +++ b/src/test/java/com/github/pjfanning/xlsx/StreamingSheetTest.java @@ -16,7 +16,6 @@ import org.junit.BeforeClass; import org.junit.Test; -import java.io.IOException; import java.io.InputStream; import java.util.Iterator; import java.util.List; @@ -110,7 +109,7 @@ public void testEmptyCellShouldHaveGeneralStyle() throws Exception { } @Test - public void testMergedRegion() throws IOException { + public void testMergedRegion() throws Exception { try (UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()) { try (XSSFWorkbook wb = new XSSFWorkbook()) { XSSFSheet sheet = wb.createSheet(); @@ -368,7 +367,7 @@ public void testGetActiveCell() throws Exception { } @Test - public void testCustomWidthAndHeight() throws IOException { + public void testCustomWidthAndHeight() throws Exception { try ( InputStream is = getInputStream("WidthsAndHeights.xlsx"); Workbook wb = StreamingReader.builder().open(is) @@ -412,7 +411,7 @@ public void testCustomWidthAndHeight() throws IOException { } @Test - public void testPaneInformation() throws IOException { + public void testPaneInformation() throws Exception { try ( XSSFWorkbook xssfWorkbook = new XSSFWorkbook(); UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get() @@ -462,7 +461,7 @@ public void testPaneInformation() throws IOException { } @Test - public void testRowStyle() throws IOException { + public void testRowStyle() throws Exception { try ( InputStream is = getInputStream("row-style.xlsx"); Workbook wb = StreamingReader.builder().open(is) @@ -484,7 +483,7 @@ public void testRowStyle() throws IOException { } @Test - public void testCellWithLineBreak() throws IOException { + public void testCellWithLineBreak() throws Exception { final String testValue = "1\n2\r\n3"; try ( XSSFWorkbook xssfWorkbook = new XSSFWorkbook(); @@ -511,7 +510,7 @@ public void testCellWithLineBreak() throws IOException { } @Test - public void testCellWithLineBreakNoSharedStrings() throws IOException { + public void testCellWithLineBreakNoSharedStrings() throws Exception { //SXSSFWorkbook does not use SharedStrings by default final String testValue = "1\n2\r\n3"; try ( diff --git a/src/test/java/com/github/pjfanning/xlsx/StreamingWorkbookTest.java b/src/test/java/com/github/pjfanning/xlsx/StreamingWorkbookTest.java index afa51743..0f8c6d4e 100644 --- a/src/test/java/com/github/pjfanning/xlsx/StreamingWorkbookTest.java +++ b/src/test/java/com/github/pjfanning/xlsx/StreamingWorkbookTest.java @@ -1,7 +1,6 @@ package com.github.pjfanning.xlsx; -import com.github.pjfanning.xlsx.exceptions.OpenException; -import com.github.pjfanning.xlsx.exceptions.ReadException; +import com.github.pjfanning.xlsx.exceptions.CheckedReadException; import com.github.pjfanning.xlsx.impl.XlsxPictureData; import fi.iki.elonen.NanoHTTPD; import org.apache.commons.io.IOUtils; @@ -430,8 +429,8 @@ public void testGetPicturesWithIssue180Xlsx() throws Exception { } } - @Test(expected = OpenException.class) - public void testEntityExpansionWithPoiDefaultSst() throws Exception { + @Test(expected = ExpectedException.class) + public void testEntityExpansionWithPoiDefaultSst() { ExploitServer.withServer(s -> fail("Should not have made request"), () -> { try( InputStream stream = getInputStream("entity-expansion-exploit-poc-file.xlsx"); @@ -445,13 +444,15 @@ public void testEntityExpansionWithPoiDefaultSst() throws Exception { System.out.println(cell.getStringCellValue()); } } - } catch(IOException e) { - throw new UncheckedIOException(e); + } catch(CheckedReadException e) { + throw new ExpectedException(e); + } catch (Exception e) { + throw new RuntimeException(e); } }); } - @Test(expected = OpenException.class) + @Test(expected = ExpectedException.class) public void testEntityExpansionWithReadOnlySst() throws Exception { ExploitServer.withServer(s -> fail("Should not have made request"), () -> { try ( @@ -465,14 +466,16 @@ public void testEntityExpansionWithReadOnlySst() throws Exception { System.out.println(cell.getStringCellValue()); } } - } catch(IOException e) { - throw new UncheckedIOException(e); + } catch(CheckedReadException e) { + throw new ExpectedException(e); + } catch (Exception e) { + throw new RuntimeException(e); } }); } @Test - public void testDataFormatter() throws IOException { + public void testDataFormatter() throws Exception { try(Workbook workbook = openWorkbook("formats.xlsx")) { Sheet sheet = workbook.getSheetAt(0); validateFormatsSheet(sheet); @@ -480,7 +483,7 @@ public void testDataFormatter() throws IOException { } @Test - public void testWithTempFileZipInputStream() throws IOException { + public void testWithTempFileZipInputStream() throws Exception { //this test cannot be run in parallel with other tests because it changes static configs ZipInputStreamZipEntrySource.setThresholdBytesForTempFiles(0); try(Workbook workbook = openWorkbook("formats.xlsx")) { @@ -492,7 +495,7 @@ public void testWithTempFileZipInputStream() throws IOException { } @Test - public void testWithTempFileZipPackage() throws IOException { + public void testWithTempFileZipPackage() throws Exception { //this test cannot be run in parallel with other tests because it changes static configs ZipPackage.setUseTempFilePackageParts(true); try(Workbook workbook = openWorkbook("formats.xlsx")) { @@ -504,7 +507,7 @@ public void testWithTempFileZipPackage() throws IOException { } @Test - public void testCallingSheetIteratorTwice() throws IOException { + public void testCallingSheetIteratorTwice() throws Exception { try(Workbook workbook = openWorkbook("formats.xlsx")) { Iterator iter1 = workbook.sheetIterator(); List sheetList1 = new ArrayList<>(); @@ -522,7 +525,7 @@ public void testCallingSheetIteratorTwice() throws IOException { } @Test - public void testCallingSheetSpliteratorTwice() throws IOException { + public void testCallingSheetSpliteratorTwice() throws Exception { try(Workbook workbook = openWorkbook("formats.xlsx")) { Spliterator iter1 = workbook.spliterator(); List sheetList1 = new ArrayList<>(); @@ -540,7 +543,7 @@ public void testCallingSheetSpliteratorTwice() throws IOException { } @Test - public void testWithGetSheetAtAndThenIterator() throws IOException { + public void testWithGetSheetAtAndThenIterator() throws Exception { try(Workbook workbook = openWorkbook("formats.xlsx")) { Sheet sheet = workbook.getSheetAt(0); validateFormatsSheet(sheet); @@ -554,7 +557,7 @@ public void testWithGetSheetAtAndThenIterator() throws IOException { } @Test - public void testNoSuchElementExceptionOnSheetIterator() throws IOException { + public void testNoSuchElementExceptionOnSheetIterator() throws Exception { try (Workbook workbook = openWorkbook("formats.xlsx")) { Iterator iter1 = workbook.sheetIterator(); assertTrue(iter1.hasNext()); @@ -565,7 +568,7 @@ public void testNoSuchElementExceptionOnSheetIterator() throws IOException { } @Test - public void testRightToLeft() throws IOException { + public void testRightToLeft() throws Exception { try( InputStream stream = getInputStream("right-to-left.xlsx"); Workbook workbook = StreamingReader.builder() @@ -593,7 +596,7 @@ public void testRightToLeft() throws IOException { } @Test - public void testRightToLeftMapBackedSst() throws IOException { + public void testRightToLeftMapBackedSst() throws Exception { try( InputStream stream = getInputStream("right-to-left.xlsx"); Workbook workbook = StreamingReader.builder() @@ -622,7 +625,7 @@ public void testRightToLeftMapBackedSst() throws IOException { } @Test - public void testAdjustLegacyCommentsDisabled() throws IOException { + public void testAdjustLegacyCommentsDisabled() throws Exception { try( InputStream stream = getInputStream("right-to-left.xlsx"); Workbook workbook = StreamingReader.builder().setReadComments(true).open(stream) @@ -638,7 +641,7 @@ public void testAdjustLegacyCommentsDisabled() throws IOException { } @Test - public void testGetErrorCellValue() throws IOException { + public void testGetErrorCellValue() throws Exception { try(UnsynchronizedByteArrayOutputStream bos = UnsynchronizedByteArrayOutputStream.builder().get()) { try(XSSFWorkbook workbook = new XSSFWorkbook()) { XSSFSheet sheet = workbook.createSheet("sheet1"); @@ -694,7 +697,7 @@ public void testBug65676() throws Exception { } @Test - public void testSheetNameCaseInsensitivity() throws IOException { + public void testSheetNameCaseInsensitivity() throws Exception { final String sheetName1 = "sheetWithCamelCaseName"; final String sheetName2 = "SHEET_WITH_CAPS_NAME"; try ( @@ -880,7 +883,7 @@ public void testExtraWhitespace() throws Exception { public void testXlsReadFailure() throws Exception { try (Workbook workbook = openWorkbook("Basic_Expense_Template_2011.xls")) { fail("Should have thrown exception"); - } catch (ReadException e) { + } catch (CheckedReadException e) { assertEquals("Unsupported File Format (only xlsx files are supported)", e.getMessage()); } } @@ -890,7 +893,7 @@ public void testXlsReadFileFailure() throws Exception { File file = new File("src/test/resources/Basic_Expense_Template_2011.xls"); try (Workbook workbook = StreamingReader.builder().open(file)) { fail("Should have thrown exception"); - } catch (ReadException e) { + } catch (CheckedReadException e) { assertEquals("Unsupported File Format (only xlsx files are supported)", e.getMessage()); } } diff --git a/src/test/java/com/github/pjfanning/xlsx/TestUtils.java b/src/test/java/com/github/pjfanning/xlsx/TestUtils.java index 774231e1..6ec9fadc 100644 --- a/src/test/java/com/github/pjfanning/xlsx/TestUtils.java +++ b/src/test/java/com/github/pjfanning/xlsx/TestUtils.java @@ -1,5 +1,6 @@ package com.github.pjfanning.xlsx; +import com.github.pjfanning.xlsx.exceptions.CheckedReadException; import org.apache.poi.ss.usermodel.*; import org.apache.poi.ss.util.CellReference; @@ -15,7 +16,7 @@ static InputStream getInputStream(String fileName) throws IOException { return TestUtils.class.getResourceAsStream("/" + fileName); } - static Workbook openWorkbook(String fileName) throws IOException { + static Workbook openWorkbook(String fileName) throws IOException, CheckedReadException { try (InputStream stream = getInputStream(fileName)) { return StreamingReader.builder() .open(stream); From a510a7af85a8e09690832050c2f25ea356bcabb3 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 15:42:33 +0100 Subject: [PATCH 2/9] preserve InvalidFormatException --- .../xlsx/impl/StreamingWorkbookReader.java | 4 ++- .../xlsx/impl/ooxml/OoxmlReader.java | 33 +++++++++---------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java index 057156e0..ab7352ca 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java @@ -12,6 +12,7 @@ import com.github.pjfanning.xlsx.impl.ooxml.OoxmlReader; import org.apache.commons.io.IOUtils; import org.apache.poi.UnsupportedFileFormatException; +import org.apache.poi.ooxml.POIXMLException; import org.apache.poi.ooxml.POIXMLProperties; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.OpenXML4JException; @@ -163,7 +164,8 @@ private OPCPackage decryptWorkbook(POIFSFileSystem poifs) throws IOException, Ge return OPCPackage.open(d.getDataStream(poifs)); } - private void loadPackage(OPCPackage pkg) throws IOException, OpenXML4JException, SAXException, XMLStreamException { + private void loadPackage(OPCPackage pkg) + throws IOException, OpenXML4JException, SAXException, XMLStreamException, POIXMLException { strictFormat = pkg.isStrictOoxmlFormat(); ooxmlReader = new OoxmlReader(builder, pkg, strictFormat); if (strictFormat) { diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java index 153b78c4..d150cbbc 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java @@ -78,7 +78,7 @@ public class OoxmlReader extends XSSFReader { @Internal public OoxmlReader(StreamingReader.Builder builder, OPCPackage pkg, boolean strictOoxmlChecksNeeded) - throws IOException, OpenXML4JException, POIXMLException { + throws IOException, OpenXML4JException, InvalidFormatException, POIXMLException { super(pkg, true); PackageRelationship coreDocRelationship = this.pkg.getRelationshipsByType( @@ -200,33 +200,30 @@ static class OoxmlSheetReader { * * @param wb package part holding workbook.xml * @throws IOException if reading the data from the package fails - * @throws POIXMLException if the package data is invalid + * @throws InvalidFormatException if the package data is invalid + * @throws RuntimeException the underlying POI code can throw other RuntimeExceptions */ OoxmlSheetReader(final StreamingReader.Builder builder, final PackagePart wb, final boolean strictOoxmlChecksNeeded) - throws IOException, POIXMLException { + throws IOException, InvalidFormatException { this.builder = builder; this.strictOoxmlChecksNeeded = strictOoxmlChecksNeeded; /* * The order of sheets is defined by the order of CTSheet elements in workbook.xml */ - try { - //step 1. Map sheet's relationship Id and the corresponding PackagePart - sheetMap = new HashMap<>(); - OPCPackage pkg = wb.getPackage(); - Set worksheetRels = getSheetRelationships(); - for (PackageRelationship rel : wb.getRelationships()) { - String relType = rel.getRelationshipType(); - if (worksheetRels.contains(relType)) { - PackagePartName relName = PackagingURIHelper.createPartName(rel.getTargetURI()); - sheetMap.put(rel.getId(), pkg.getPart(relName)); - } + //step 1. Map sheet's relationship Id and the corresponding PackagePart + sheetMap = new HashMap<>(); + OPCPackage pkg = wb.getPackage(); + Set worksheetRels = getSheetRelationships(); + for (PackageRelationship rel : wb.getRelationships()) { + String relType = rel.getRelationshipType(); + if (worksheetRels.contains(relType)) { + PackagePartName relName = PackagingURIHelper.createPartName(rel.getTargetURI()); + sheetMap.put(rel.getId(), pkg.getPart(relName)); } - //step 2. Read array of CTSheet elements, wrap it in a LinkedList - sheetRefList = createSheetListFromWB(wb); - } catch (InvalidFormatException e) { - throw new POIXMLException(e); } + //step 2. Read array of CTSheet elements, wrap it in a LinkedList + sheetRefList = createSheetListFromWB(wb); } int size() { From faa441242892db51e79db75cf8e7f4cc65b974b3 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 15:55:48 +0100 Subject: [PATCH 3/9] update javadoc --- .../pjfanning/xlsx/exceptions/CheckedReadException.java | 6 ++++++ .../github/pjfanning/xlsx/exceptions/ReadException.java | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java b/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java index f4e9fb7a..06d93e0f 100644 --- a/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java +++ b/src/main/java/com/github/pjfanning/xlsx/exceptions/CheckedReadException.java @@ -1,5 +1,11 @@ package com.github.pjfanning.xlsx.exceptions; +/** + * An exception that is thrown if there is a problem reading the Excel file. + * + * @see ReadException + * @since 5.0.0 + */ public class CheckedReadException extends ExcelCheckedException { public CheckedReadException() { diff --git a/src/main/java/com/github/pjfanning/xlsx/exceptions/ReadException.java b/src/main/java/com/github/pjfanning/xlsx/exceptions/ReadException.java index 5ff14a72..2f1baa61 100644 --- a/src/main/java/com/github/pjfanning/xlsx/exceptions/ReadException.java +++ b/src/main/java/com/github/pjfanning/xlsx/exceptions/ReadException.java @@ -1,5 +1,14 @@ package com.github.pjfanning.xlsx.exceptions; +/** + * A Runtime Exception that is thrown if there is a problem reading the Excel file. + * This is used in APIs where the method is unable to throw a checked exception. + *

+ * Before v5.0.0, this exception was used in more places but {@link CheckedReadException} is now used + * where checked exceptions are allowed. + *

+ * @see CheckedReadException + */ public class ReadException extends ExcelRuntimeException { public ReadException() { From 96110fa48cb1b9c78c31c452bd8c5a4996a9e19a Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 15:57:57 +0100 Subject: [PATCH 4/9] Update StreamingWorkbookReader.java --- .../github/pjfanning/xlsx/impl/StreamingWorkbookReader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java index ab7352ca..3e2b5b82 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java @@ -90,10 +90,10 @@ public void init(InputStream is) throws IOException, CheckedReadException { throw e; } catch(GeneralSecurityException e) { IOUtils.closeQuietly(pkg); - throw new ReadException("Unable to read workbook - Decryption failed", e); + throw new CheckedReadException("Unable to read workbook - Decryption failed", e); } catch(OpenXML4JException | XMLStreamException | RuntimeException e) { IOUtils.closeQuietly(pkg); - throw new ReadException("Unable to read workbook", e); + throw new CheckedReadException("Unable to read workbook", e); } } else { File f = null; From 47a15a39da6cf9bd9c0c5eb1ccbdfda56f764a2b Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 16:02:10 +0100 Subject: [PATCH 5/9] use CheckedReadException --- .../pjfanning/xlsx/impl/StreamingWorkbookReader.java | 2 +- .../github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java index 3e2b5b82..ef3a22d8 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingWorkbookReader.java @@ -165,7 +165,7 @@ private OPCPackage decryptWorkbook(POIFSFileSystem poifs) throws IOException, Ge } private void loadPackage(OPCPackage pkg) - throws IOException, OpenXML4JException, SAXException, XMLStreamException, POIXMLException { + throws IOException, CheckedReadException, OpenXML4JException, SAXException, XMLStreamException, POIXMLException { strictFormat = pkg.isStrictOoxmlFormat(); ooxmlReader = new OoxmlReader(builder, pkg, strictFormat); if (strictFormat) { diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java index d150cbbc..6ca88ed7 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java @@ -22,6 +22,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more import com.github.pjfanning.poi.xssf.streaming.TempFileSharedStringsTable; import com.github.pjfanning.xlsx.CommentsImplementationType; import com.github.pjfanning.xlsx.StreamingReader; +import com.github.pjfanning.xlsx.exceptions.CheckedReadException; import com.github.pjfanning.xlsx.exceptions.MissingSheetException; import com.github.pjfanning.xlsx.exceptions.OpenException; import org.apache.poi.ooxml.POIXMLException; @@ -78,7 +79,7 @@ public class OoxmlReader extends XSSFReader { @Internal public OoxmlReader(StreamingReader.Builder builder, OPCPackage pkg, boolean strictOoxmlChecksNeeded) - throws IOException, OpenXML4JException, InvalidFormatException, POIXMLException { + throws IOException, CheckedReadException, OpenXML4JException, POIXMLException { super(pkg, true); PackageRelationship coreDocRelationship = this.pkg.getRelationshipsByType( @@ -205,7 +206,7 @@ static class OoxmlSheetReader { */ OoxmlSheetReader(final StreamingReader.Builder builder, final PackagePart wb, final boolean strictOoxmlChecksNeeded) - throws IOException, InvalidFormatException { + throws IOException, CheckedReadException, InvalidFormatException { this.builder = builder; this.strictOoxmlChecksNeeded = strictOoxmlChecksNeeded; /* @@ -261,7 +262,7 @@ SheetData getSheetData(final XSSFSheetRef sheetRef) { return sd; } - private ArrayList createSheetListFromWB(PackagePart wb) throws IOException, OpenException, POIXMLException { + private ArrayList createSheetListFromWB(PackagePart wb) throws IOException, CheckedReadException, POIXMLException { XMLSheetRefReader xmlSheetRefReader = new XMLSheetRefReader(); XMLReader xmlReader; @@ -274,7 +275,7 @@ private ArrayList createSheetListFromWB(PackagePart wb) throws IOE try (InputStream stream = wb.getInputStream()) { xmlReader.parse(new InputSource(stream)); } catch (SAXException e) { - throw new OpenException(e); + throw new CheckedReadException(e); } final ArrayList validSheets = new ArrayList<>(); From b3cec7b5c807c495590febfff83c643a6e201a24 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 16:37:25 +0100 Subject: [PATCH 6/9] make class package private --- .../xlsx/impl/ooxml/OoXmlStrictConverterUtils.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoXmlStrictConverterUtils.java b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoXmlStrictConverterUtils.java index d84ec8d3..241c5bfd 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoXmlStrictConverterUtils.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoXmlStrictConverterUtils.java @@ -1,26 +1,26 @@ package com.github.pjfanning.xlsx.impl.ooxml; import com.github.pjfanning.xlsx.exceptions.ReadException; -import org.apache.poi.util.Beta; +import org.apache.poi.util.Internal; import java.io.*; import java.nio.charset.StandardCharsets; import java.util.Properties; -@Beta -public class OoXmlStrictConverterUtils { +@Internal +final class OoXmlStrictConverterUtils { private OoXmlStrictConverterUtils() {} - public static boolean isBlank(final String str) { + static boolean isBlank(final String str) { return str == null || str.trim().length() == 0; } - public static boolean isNotBlank(final String str) { + static boolean isNotBlank(final String str) { return !isBlank(str); } - public static Properties readMappings() throws ReadException { + static Properties readMappings() throws ReadException { Properties props = new Properties(); try(InputStream is = OoXmlStrictConverterUtils.class.getResourceAsStream("/ooxml-strict-mappings.properties"); BufferedReader reader = new BufferedReader(new InputStreamReader(is, StandardCharsets.ISO_8859_1))) { From 79d40e897d756b4436d9651e15da39732c5b2215 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 21:24:59 +0100 Subject: [PATCH 7/9] Update ExcelCheckedException.java --- .../github/pjfanning/xlsx/exceptions/ExcelCheckedException.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java b/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java index 22cca68c..7869e5de 100644 --- a/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java +++ b/src/main/java/com/github/pjfanning/xlsx/exceptions/ExcelCheckedException.java @@ -2,6 +2,8 @@ /** * A parent class for all the excel-streaming-reader specific Checked Exceptions (i.e. not Runtime Exceptions). + * + * @since 5.0.0 */ public class ExcelCheckedException extends Exception { From 58c1b6af9e99cec73109aca2ce40b159276ce81b Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 21:39:22 +0100 Subject: [PATCH 8/9] update getRow --- .../xlsx/impl/StreamingRowIterator.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingRowIterator.java b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingRowIterator.java index 1fe0bbd1..974d68ac 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/StreamingRowIterator.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/StreamingRowIterator.java @@ -89,7 +89,7 @@ class StreamingRowIterator implements CloseableIterator { final Set hiddenColumns, final Map columnWidths, final Set mergedCells, final Set hyperlinks, final Map sharedFormulaMap, final float defaultRowHeight, - final StreamingSheet sheet) throws ParseException { + final StreamingSheet sheet) throws XMLStreamException { this.streamingSheetReader = streamingSheetReader; this.sst = sst; this.stylesTable = stylesTable; @@ -118,17 +118,13 @@ private float getDefaultRowHeight() { * * @return true if data was read */ - private boolean getRow() throws ParseException { - try { - rowCache.clear(); - while(rowCache.size() < rowCacheSize && parser.hasNext()) { - handleEvent(parser.nextEvent()); - } - rowCacheIterator = rowCache.iterator(); - return rowCacheIterator.hasNext(); - } catch(XMLStreamException e) { - throw new ParseException("Error reading XML stream", e); + private boolean getRow() throws XMLStreamException { + rowCache.clear(); + while(rowCache.size() < rowCacheSize && parser.hasNext()) { + handleEvent(parser.nextEvent()); } + rowCacheIterator = rowCache.iterator(); + return rowCacheIterator.hasNext(); } private void handleEvent(XMLEvent event) { @@ -630,9 +626,17 @@ private String getAttributeValue(Attribute att) { return att == null ? null : att.getValue(); } + /** + * @return whether there are more rows + * @throws ParseException if there is an error parsing the underlying data + */ @Override public boolean hasNext() throws ParseException { - return (rowCacheIterator != null && rowCacheIterator.hasNext()) || getRow(); + try { + return (rowCacheIterator != null && rowCacheIterator.hasNext()) || getRow(); + } catch (XMLStreamException e) { + throw new ParseException(e); + } } @Override From 4bd6587f6ec5789fc5a4e5ac770b02e7833fd4df Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 19 Jan 2024 22:29:49 +0100 Subject: [PATCH 9/9] Update OoxmlReader.java --- .../pjfanning/xlsx/impl/ooxml/OoxmlReader.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java index 6ca88ed7..df44d02d 100644 --- a/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java +++ b/src/main/java/com/github/pjfanning/xlsx/impl/ooxml/OoxmlReader.java @@ -24,7 +24,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more import com.github.pjfanning.xlsx.StreamingReader; import com.github.pjfanning.xlsx.exceptions.CheckedReadException; import com.github.pjfanning.xlsx.exceptions.MissingSheetException; -import com.github.pjfanning.xlsx.exceptions.OpenException; import org.apache.poi.ooxml.POIXMLException; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.OpenXML4JException; @@ -73,13 +72,13 @@ public class OoxmlReader extends XSSFReader { * @param pkg The package this poi filesystem reader will use * @param strictOoxmlChecksNeeded whether to perform strict OOXML checks * @throws IOException if an error occurs while reading the package - * @throws OpenXML4JException if an error occurs while parsing the package - * @throws POIXMLException if the package is invalid + * @throws CheckedReadException if an error occurs while reading the package + * @throws OpenXML4JException if there is an error parsing the OpenXML file */ @Internal public OoxmlReader(StreamingReader.Builder builder, OPCPackage pkg, boolean strictOoxmlChecksNeeded) - throws IOException, CheckedReadException, OpenXML4JException, POIXMLException { + throws IOException, CheckedReadException, OpenXML4JException { super(pkg, true); PackageRelationship coreDocRelationship = this.pkg.getRelationshipsByType( @@ -93,13 +92,17 @@ public OoxmlReader(StreamingReader.Builder builder, PackageRelationshipTypes.STRICT_CORE_DOCUMENT).getRelationship(0); if (coreDocRelationship == null) { - throw new POIXMLException("OOXML file structure broken/invalid - no core document found!"); + throw new CheckedReadException("OOXML file structure broken/invalid - no core document found!"); } } // Get the part that holds the workbook workbookPart = this.pkg.getPart(coreDocRelationship); - ooxmlSheetReader = new OoxmlSheetReader(builder, workbookPart, strictOoxmlChecksNeeded); + try { + ooxmlSheetReader = new OoxmlSheetReader(builder, workbookPart, strictOoxmlChecksNeeded); + } catch (InvalidFormatException e) { + throw new CheckedReadException("invalid format", e); + } }