Copilot commented on code in PR #3377:
URL: https://github.com/apache/tika/pull/3377#discussion_r4336618755
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pkg-module/src/main/java/org/apache/tika/parser/pkg/RarParser.java:
##########
@@ -112,4 +119,24 @@ public void parse(TikaInputStream tis, ContentHandler
handler, Metadata metadata
xhtml.endDocument();
}
+
+ /**
+ * junrar 8.1.2+ reports a failed entry (truncated archive, bad CRC,
missing password) as an
+ * IOException on the entry's stream. End the entry there instead, as
junrar 8.1.1 did, so
+ * the bytes it delivered are still parsed and the rest of the archive is
reached.
+ */
+ private static class EntryStream extends ProxyInputStream {
+ private IOException failure;
+
+ EntryStream(InputStream in) {
+ super(in);
+ }
+
+ @Override
+ protected void handleIOException(IOException e) {
+ if (failure == null) {
+ failure = e;
+ }
+ }
Review Comment:
If multiple `IOException`s occur on the same entry stream (e.g., repeated
reads after failure, or an additional exception on close), only the first is
retained and subsequent ones are silently dropped. Consider preserving
additional exceptions by adding them as suppressed to the first captured
exception (e.g., when `failure != null`, call `failure.addSuppressed(e)`), so
diagnostics aren’t lost.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pkg-module/src/test/java/org/apache/tika/parser/pkg/RarParserTest.java:
##########
@@ -92,6 +99,25 @@ public void testEmbedded() throws Exception {
assertContains("test-documents/testXML.xml", content);
}
+ @Test
+ public void testCorruptEntryKeepsLaterEntries(@TempDir Path tmp) throws
Exception {
+ byte[] rar;
+ try (TikaInputStream tis =
getResourceAsStream("/test-documents/test-documents.rar")) {
+ rar = IOUtils.toByteArray(tis);
+ }
+ // testEXCEL.xls, the first entry, has its packed data at 80..2885
+ rar[1500] ^= 0x5A;
+ Path corrupt = tmp.resolve("corrupt.rar");
+ Files.write(corrupt, rar);
Review Comment:
The test corrupts the archive using a hard-coded byte offset (1500) derived
from current test file internals, which is brittle if the RAR file changes
(even slightly) or is regenerated. Consider corrupting in a way that is
resilient to file layout changes (e.g., truncating the archive by a fixed
amount from the end, or locating the packed-data region programmatically for
the specific entry name before flipping a byte).
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pkg-module/src/test/java/org/apache/tika/parser/pkg/RarParserTest.java:
##########
@@ -92,6 +99,25 @@ public void testEmbedded() throws Exception {
assertContains("test-documents/testXML.xml", content);
}
+ @Test
+ public void testCorruptEntryKeepsLaterEntries(@TempDir Path tmp) throws
Exception {
+ byte[] rar;
+ try (TikaInputStream tis =
getResourceAsStream("/test-documents/test-documents.rar")) {
+ rar = IOUtils.toByteArray(tis);
+ }
+ // testEXCEL.xls, the first entry, has its packed data at 80..2885
+ rar[1500] ^= 0x5A;
+ Path corrupt = tmp.resolve("corrupt.rar");
+ Files.write(corrupt, rar);
+
+ List<Metadata> metadataList = getRecursiveMetadata(corrupt, new
ParseContext(), false);
+
assertNotNull(metadataList.get(0).get(TikaCoreProperties.TIKA_META_EXCEPTION_EMBEDDED_STREAM));
+ assertContains("test-documents/testXML.xml",
+ metadataList.get(0).get(TikaCoreProperties.TIKA_CONTENT));
+ assertEquals("/testXML.xml",
+ metadataList.get(metadataList.size() -
1).get(TikaCoreProperties.EMBEDDED_RESOURCE_PATH));
Review Comment:
The assertion that the last metadata item is `\"/testXML.xml\"` is
order-dependent and may be flaky if embedded extraction order changes (or if
additional embedded items are added). A more robust check is to search
`metadataList` for an item whose `EMBEDDED_RESOURCE_PATH` equals
`\"/testXML.xml\"` and assert it exists (and/or assert that a later entry than
the corrupted one is present), rather than assuming it is the final element.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
