[
https://issues.apache.org/jira/browse/TIKA-4985?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18129604#comment-18129604
]
ASF GitHub Bot commented on TIKA-4985:
--------------------------------------
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.
> Prep for 4.2.0 release
> ----------------------
>
> Key: TIKA-4985
> URL: https://issues.apache.org/jira/browse/TIKA-4985
> Project: Tika
> Issue Type: Task
> Reporter: Tim Allison
> Priority: Major
>
> From the first round: junrar changed exception handling. The updated zip bomb
> detection triggers on compressed log files, and a few other fixes. Vast
> majority of content is unchanged. the mhtml change didn't do much. The html
> detection change is a clear improvement.
>
> Will run round two later today and post results if they're good enough for an
> rc1.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
