Skip to content

Commit 4ead960

Browse files
tballisontballison
authored
TIKA-4879: carry on safer get missing part in xlsx and vsdx (#3135)
1 parent bea45c9 commit 4ead960

4 files changed

Lines changed: 165 additions & 4 deletions

File tree

‎CHANGES.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
Release 4.1.0 - unreleased
22

3+
* A missing OOXML relationship target no longer aborts the whole file:
4+
the threaded-comment and person lookups in xlsx and the page lookups in
5+
vsdx went straight to POI's getRelatedPart, whose unchecked
6+
IllegalArgumentException surfaced as "Error creating OOXML extractor" and
7+
dropped the text already extracted. They route through
8+
safeGetRelatedPart, as branch_3x already did (TIKA-4879).
9+
310
* RawTiffDetector rejects a BigTIFF directory offset near Long.MAX_VALUE
411
instead of letting the bounds check overflow. Adding the entry-count
512
size to such an offset wrapped negative and read as "already in the

‎tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/ooxml/VSDXExtractorDecorator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ private List<PackagePart> getPageParts() throws InvalidFormatException {
9090
PackageRelationshipCollection pageRels =
9191
pagesPart.getRelationshipsByType(VISIO_PAGE_REL);
9292
for (PackageRelationship rel : pageRels) {
93-
PackagePart pagePart = pagesPart.getRelatedPart(rel);
93+
PackagePart pagePart = safeGetRelatedPart(pagesPart, rel);
9494
if (pagePart != null) {
9595
pageParts.add(pagePart);
9696
}
@@ -113,7 +113,7 @@ private PackagePart getRelatedPart(PackagePart part, String relType)
113113
if (rels.isEmpty()) {
114114
return null;
115115
}
116-
return part.getRelatedPart(rels.getRelationship(0));
116+
return safeGetRelatedPart(part, rels.getRelationship(0));
117117
}
118118

119119
@Override

‎tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/ooxml/XSSFExcelExtractorDecorator.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ private void getThreadedComments(OPCPackage container, PackagePart sheetPart, XH
663663
return;
664664
}
665665
for (PackageRelationship rel : coll) {
666-
PackagePart threadedCommentPart = sheetPart.getRelatedPart(rel);
666+
PackagePart threadedCommentPart = safeGetRelatedPart(sheetPart, rel);
667667
if (threadedCommentPart == null) {
668668
continue;
669669
}
@@ -690,7 +690,7 @@ private void getPersons(OPCPackage container, Metadata metadata) throws TikaExce
690690
return;
691691
}
692692
for (PackageRelationship rel : coll) {
693-
PackagePart personsPart = workbookPart.getRelatedPart(rel);
693+
PackagePart personsPart = safeGetRelatedPart(workbookPart, rel);
694694
if (personsPart == null) {
695695
continue;
696696
}

‎tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/test/java/org/apache/tika/parser/microsoft/ooxml/OOXMLMissingRelatedPartTest.java‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.tika.parser.microsoft.ooxml;
18+
19+
import static org.junit.jupiter.api.Assertions.assertEquals;
20+
import static org.junit.jupiter.api.Assertions.assertNotNull;
21+
22+
import java.io.ByteArrayOutputStream;
23+
import java.nio.charset.StandardCharsets;
24+
import java.util.List;
25+
26+
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
27+
import org.apache.commons.compress.archivers.zip.ZipArchiveInputStream;
28+
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
29+
import org.apache.commons.io.IOUtils;
30+
import org.junit.jupiter.api.Test;
31+
32+
import org.apache.tika.TikaTest;
33+
import org.apache.tika.io.TikaInputStream;
34+
import org.apache.tika.metadata.HttpHeaders;
35+
import org.apache.tika.metadata.Metadata;
36+
import org.apache.tika.metadata.TikaCoreProperties;
37+
38+
/**
39+
* An OOXML package may declare a relationship whose target part is missing -- a truncated
40+
* or otherwise malformed file. POI's {@code PackagePart.getRelatedPart} then throws an
41+
* unchecked {@code IllegalArgumentException}, which
42+
* {@link OOXMLExtractorFactory} converts into a TikaException: the whole file aborts and
43+
* even the text already extracted is lost. Every such call must go through
44+
* {@link AbstractOOXMLExtractor#safeGetRelatedPart}.
45+
*
46+
* <p>Same failure class as the 3.3.2 docx regression (740 files crashed on a missing
47+
* numbering.xml/settings.xml). 3.3.2 guards these four xlsx/vsdx sites; 4.0.0 shipped
48+
* without them (TIKA-4879).
49+
*/
50+
public class OOXMLMissingRelatedPartTest extends TikaTest {
51+
52+
private static final String XLSX_TYPE =
53+
"application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
54+
private static final String VSDX_TYPE = "application/vnd.ms-visio.drawing";
55+
56+
@Test
57+
public void testDanglingThreadedCommentRelationship() throws Exception {
58+
//XSSFExcelExtractorDecorator.getThreadedComments
59+
byte[] xlsx = withExtraRelationship("testComment.xlsx",
60+
"xl/worksheets/_rels/sheet1.xml.rels",
61+
"http://schemas.microsoft.com/office/2017/10/relationships/threadedComment",
62+
"../threadedComments/threadedComment1.xml");
63+
assertSheetTextSurvives(xlsx);
64+
}
65+
66+
@Test
67+
public void testDanglingPersonRelationship() throws Exception {
68+
//XSSFExcelExtractorDecorator.getPersons
69+
byte[] xlsx = withExtraRelationship("testComment.xlsx", "xl/_rels/workbook.xml.rels",
70+
"http://schemas.microsoft.com/office/2017/10/relationships/person",
71+
"persons/person.xml");
72+
assertSheetTextSurvives(xlsx);
73+
}
74+
75+
@Test
76+
public void testMissingVisioPage() throws Exception {
77+
//VSDXExtractorDecorator.getPageParts, the per-page loop
78+
assertVisioParseCompletes(withoutEntry("testVISIO.vsdx", "visio/pages/page1.xml"));
79+
}
80+
81+
@Test
82+
public void testMissingVisioPagesPart() throws Exception {
83+
//VSDXExtractorDecorator.getRelatedPart(PackagePart, String), document.xml -> pages.xml
84+
assertVisioParseCompletes(withoutEntry("testVISIO.vsdx", "visio/pages/pages.xml"));
85+
}
86+
87+
/**
88+
* The pages are unreachable, but the parse must still run to completion: the EMF
89+
* thumbnail is emitted after buildXHTML, so its presence proves we did not abort.
90+
*/
91+
private void assertVisioParseCompletes(byte[] vsdx) throws Exception {
92+
List<Metadata> metadataList = assertParses(vsdx, VSDX_TYPE);
93+
assertEquals(2, metadataList.size());
94+
assertEquals("image/emf", metadataList.get(1).get(HttpHeaders.CONTENT_TYPE));
95+
}
96+
97+
/** The dangling relationship must not cost us the sheet text that parsed fine. */
98+
private void assertSheetTextSurvives(byte[] xlsx) throws Exception {
99+
List<Metadata> metadataList = assertParses(xlsx, XLSX_TYPE);
100+
assertEquals(1, metadataList.size());
101+
assertContains("Here is some text",
102+
metadataList.get(0).get(TikaCoreProperties.TIKA_CONTENT));
103+
}
104+
105+
private List<Metadata> assertParses(byte[] bytes, String expectedType) throws Exception {
106+
List<Metadata> metadataList;
107+
try (TikaInputStream tis = TikaInputStream.get(bytes)) {
108+
//suppressException=false: an escaping IllegalArgumentException fails the test here
109+
metadataList = getRecursiveMetadata(tis, false);
110+
}
111+
Metadata m = metadataList.get(0);
112+
assertEquals(expectedType, m.get(HttpHeaders.CONTENT_TYPE));
113+
assertNotNull(m.get(TikaCoreProperties.TIKA_CONTENT));
114+
return metadataList;
115+
}
116+
117+
/** Copies the resource, omitting one zip entry and leaving its relationship dangling. */
118+
private byte[] withoutEntry(String resource, String entryName) throws Exception {
119+
return copy(resource, entryName, null, null, null);
120+
}
121+
122+
/** Copies the resource, appending a relationship whose target is not in the package. */
123+
private byte[] withExtraRelationship(String resource, String relsEntry, String type,
124+
String target) throws Exception {
125+
return copy(resource, null, relsEntry, type, target);
126+
}
127+
128+
private byte[] copy(String resource, String dropEntry, String relsEntry, String type,
129+
String target) throws Exception {
130+
ByteArrayOutputStream bos = new ByteArrayOutputStream();
131+
try (ZipArchiveInputStream zin =
132+
new ZipArchiveInputStream(getResourceAsStream("/test-documents/" + resource));
133+
ZipArchiveOutputStream zout = new ZipArchiveOutputStream(bos)) {
134+
ZipArchiveEntry entry;
135+
while ((entry = zin.getNextEntry()) != null) {
136+
if (entry.getName().equals(dropEntry)) {
137+
continue;
138+
}
139+
byte[] data = IOUtils.toByteArray(zin);
140+
if (entry.getName().equals(relsEntry)) {
141+
String rels = new String(data, StandardCharsets.UTF_8);
142+
String injected = "<Relationship Id=\"rIdMissingTarget\" Type=\"" + type +
143+
"\" Target=\"" + target + "\"/></Relationships>";
144+
data = rels.replace("</Relationships>", injected)
145+
.getBytes(StandardCharsets.UTF_8);
146+
}
147+
zout.putArchiveEntry(new ZipArchiveEntry(entry.getName()));
148+
zout.write(data);
149+
zout.closeArchiveEntry();
150+
}
151+
}
152+
return bos.toByteArray();
153+
}
154+
}

0 commit comments

Comments
 (0)