Skip to content

Commit c87f1ae

Browse files
tballisontballison
authored
clean up metadata safe (#2980)
1 parent c42b108 commit c87f1ae

17 files changed

Lines changed: 70 additions & 99 deletions

File tree

‎tika-core/src/main/java/org/apache/tika/metadata/Metadata.java‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ private String[] appendValues(String[] values, final String value) {
100100

101101

102102
private MetadataWriteLimiter writeLimiter = ACCEPT_ALL;
103-
private transient boolean trusted;
104103
/**
105104
* Constructs a new, empty metadata.
106105
*/
@@ -312,30 +311,22 @@ public void add(final String name, final String value) {
312311
if (blockReservedKeyWrite(name)) {
313312
return;
314313
}
315-
addUnchecked(name, value);
316-
}
317-
318-
/** Trusted add, bypassing the reserved-key guard. */
319-
private void addUnchecked(final String name, final String value) {
320-
writeLimiter.add(name, value, metadata);
314+
addTrusted(name, value);
321315
}
322316

323317
/**
324-
* Mark this Metadata as a trusted transformation target (e.g. a metadata filter), letting
325-
* String writes reach reserved Tika-native ({@code tk:}) keys. Reset when the transformation
326-
* is done.
318+
* Trusted add, bypassing the reserved-key guard: writes reach reserved Tika-native
319+
* ({@code tk:}) keys directly. For internal/known-trusted writers (metadata filters,
320+
* clone/merge/deserialize, emit-time enrichment) that legitimately need to assert a
321+
* reserved key by name rather than by its {@link Property}.
327322
*/
328-
public void setTrusted(boolean trusted) {
329-
this.trusted = trusted;
330-
}
331-
332-
public boolean isTrusted() {
333-
return trusted;
323+
public void addTrusted(final String name, final String value) {
324+
writeLimiter.add(name, value, metadata);
334325
}
335326

336-
/** Drop String writes to reserved Tika-native keys unless trusted; use their Property. */
327+
/** Drop String writes to reserved Tika-native keys; use their Property or {@link #addTrusted}/{@link #setTrusted(String, String)}. */
337328
private boolean blockReservedKeyWrite(String name) {
338-
if (!trusted && ReservedNamespaces.isTikaNative(name)) {
329+
if (ReservedNamespaces.isTikaNative(name)) {
339330
LOG.debug("Dropping String write to reserved metadata key '{}'; use its Property.", name);
340331
return true;
341332
}
@@ -357,9 +348,9 @@ public void reconstruct(String name, String value, boolean append) {
357348
set(property, value);
358349
}
359350
} else if (append) {
360-
addUnchecked(name, value);
351+
addTrusted(name, value);
361352
} else {
362-
setUnchecked(name, value);
353+
setTrusted(name, value);
363354
}
364355
return;
365356
}
@@ -383,7 +374,7 @@ protected void add(final String name, final String[] newValues) {
383374
set(name, newValues);
384375
} else {
385376
for (String val : newValues) {
386-
addUnchecked(name, val);
377+
addTrusted(name, val);
387378
}
388379
}
389380
}
@@ -414,7 +405,7 @@ public void add(final Property property, final String value) {
414405
set(property, value);
415406
} else {
416407
if (property.isMultiValuePermitted()) {
417-
addUnchecked(property.getName(), value);
408+
addTrusted(property.getName(), value);
418409
} else {
419410
throw new PropertyTypeException(
420411
property.getName() + " : " + property.getPropertyType());
@@ -450,11 +441,14 @@ public void set(String name, String value) {
450441
if (blockReservedKeyWrite(name)) {
451442
return;
452443
}
453-
setUnchecked(name, value);
444+
setTrusted(name, value);
454445
}
455446

456-
/** Trusted set, bypassing the reserved-key guard. */
457-
private void setUnchecked(String name, String value) {
447+
/**
448+
* Trusted set, bypassing the reserved-key guard: writes reach reserved Tika-native
449+
* ({@code tk:}) keys directly. See {@link #addTrusted}.
450+
*/
451+
public void setTrusted(String name, String value) {
458452
writeLimiter.set(name, value, metadata);
459453
}
460454

@@ -464,7 +458,7 @@ protected void set(String name, String[] values) {
464458
if (values != null) {
465459
metadata.remove(name);
466460
for (String v : values) {
467-
addUnchecked(name, v);
461+
addTrusted(name, v);
468462
}
469463
} else {
470464
metadata.remove(name);
@@ -490,7 +484,7 @@ public void set(Property property, String value) {
490484
}
491485
}
492486
} else {
493-
setUnchecked(property.getName(), value);
487+
setTrusted(property.getName(), value);
494488
}
495489
}
496490

‎tika-core/src/main/java/org/apache/tika/metadata/filter/CaptureGroupMetadataFilter.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ protected void filter(Metadata metadata) {
112112
}
113113
Matcher m = regex.matcher(val);
114114
if (m.find()) {
115-
metadata.set(targetField, m.group(1));
115+
// target field name is operator-configured; may legitimately be a reserved key
116+
metadata.setTrusted(targetField, m.group(1));
116117
}
117118
}
118119

‎tika-core/src/main/java/org/apache/tika/metadata/filter/FieldNameMappingFilter.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ protected void filter(Metadata metadata) {
7070
String[] vals = metadata.getValues(n);
7171
metadata.remove(n);
7272
for (String val : vals) {
73-
metadata.add(mappings.get(n), val);
73+
// target field name is operator-configured; may legitimately be a reserved key
74+
metadata.addTrusted(mappings.get(n), val);
7475
}
7576
} else {
7677
metadata.remove(n);
@@ -82,7 +83,7 @@ protected void filter(Metadata metadata) {
8283
String[] vals = metadata.getValues(n);
8384
metadata.remove(n);
8485
for (String val : vals) {
85-
metadata.add(mappings.get(n), val);
86+
metadata.addTrusted(mappings.get(n), val);
8687
}
8788
}
8889
}

‎tika-core/src/main/java/org/apache/tika/metadata/filter/GeoPointMetadataFilter.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ protected void filter(Metadata metadata) {
8888
if (StringUtils.isEmpty(lng)) {
8989
return;
9090
}
91-
metadata.set(geoPointFieldName, lat + "," + lng);
91+
// target field name is operator-configured; may legitimately be a reserved key
92+
metadata.setTrusted(geoPointFieldName, lat + "," + lng);
9293
}
9394
}

‎tika-core/src/main/java/org/apache/tika/metadata/filter/MetadataFilter.java‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@
1919
import java.io.Closeable;
2020
import java.io.IOException;
2121
import java.io.Serializable;
22-
import java.util.IdentityHashMap;
2322
import java.util.List;
24-
import java.util.Map;
2523

2624
import org.apache.tika.exception.TikaException;
2725
import org.apache.tika.metadata.Metadata;
@@ -41,21 +39,14 @@ public abstract class MetadataFilter implements Serializable, Closeable {
4139
*/
4240
public final void filter(List<Metadata> metadataList, ParseContext parseContext)
4341
throws TikaException {
44-
Map<Metadata, Boolean> previous = new IdentityHashMap<>();
45-
for (Metadata m : metadataList) {
46-
previous.put(m, m.isTrusted());
47-
m.setTrusted(true);
48-
}
49-
try {
50-
doFilter(metadataList, parseContext);
51-
} finally {
52-
previous.forEach(Metadata::setTrusted);
53-
}
42+
doFilter(metadataList, parseContext);
5443
}
5544

5645
/**
57-
* Applies the filter in place. Reserved-key String writes are permitted here: filters run
58-
* on already-parsed, trusted metadata.
46+
* Applies the filter in place. A filter that needs to write a reserved Tika-native
47+
* ({@code tk:}) key by name (rather than via its {@link org.apache.tika.metadata.Property})
48+
* should call {@link Metadata#addTrusted} / {@link Metadata#setTrusted} explicitly at that
49+
* write.
5950
*
6051
* @param metadataList the list to filter (must be mutable)
6152
* @param parseContext per-request context

‎tika-core/src/test/java/org/apache/tika/metadata/MetadataInternalKeyGuardTest.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,22 @@ public void testReconstructPreservesUnregisteredReservedKey() {
121121
}
122122

123123
@Test
124-
public void testTrustedModeAllowsReservedStringWrites() {
124+
public void testTrustedWriteBypassesGuard() {
125125
Metadata metadata = new Metadata();
126-
metadata.setTrusted(true);
127-
metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "trusted");
126+
metadata.setTrusted(TikaCoreProperties.TIKA_CONTENT.getName(), "trusted");
128127
assertEquals("trusted", metadata.get(TikaCoreProperties.TIKA_CONTENT));
129128

130-
metadata.setTrusted(false);
129+
// untrusted String-path attempt must not clobber
131130
metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "blocked");
132131
assertEquals("trusted", metadata.get(TikaCoreProperties.TIKA_CONTENT));
133132
}
133+
134+
@Test
135+
public void testTrustedAddBypassesGuard() {
136+
Metadata metadata = new Metadata();
137+
metadata.addTrusted(TikaCoreProperties.TIKA_PARSED_BY.getName(), "p1");
138+
metadata.addTrusted(TikaCoreProperties.TIKA_PARSED_BY.getName(), "p2");
139+
assertArrayEquals(new String[] {"p1", "p2"},
140+
metadata.getValues(TikaCoreProperties.TIKA_PARSED_BY));
141+
}
134142
}

‎tika-eval/tika-eval-app/src/main/java/org/apache/tika/eval/app/io/ExtractReader.java‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -247,21 +247,16 @@ private List<Metadata> generateListFromTextFile(Reader reader, FileSuffixes file
247247
* real diffs, not the rename. Harmless on 4.0 extracts: the legacy keys are simply absent.
248248
*/
249249
private static void normalizeLegacyKeys(Metadata m) {
250-
m.setTrusted(true); // sanctioned trusted transformation: may write reserved tk: keys
251-
try {
252-
for (Map.Entry<String, String> e : LEGACY_KEY_MAP.entrySet()) {
253-
remapLegacyKey(m, e.getKey(), e.getValue());
254-
}
255-
// digest keys: X-TIKA:digest:<alg> -> tk:digest:<alg> (algorithm unchanged; MD5 drives
256-
// embedded-doc matching). names() is a snapshot, so remapping while iterating is safe.
257-
for (String name : m.names()) {
258-
if (name.startsWith(LEGACY_DIGEST_PREFIX)) {
259-
remapLegacyKey(m, name, TikaCoreProperties.TIKA_META_PREFIX
260-
+ name.substring(TikaCoreProperties.LEGACY_TIKA_META_PREFIX.length()));
261-
}
250+
for (Map.Entry<String, String> e : LEGACY_KEY_MAP.entrySet()) {
251+
remapLegacyKey(m, e.getKey(), e.getValue());
252+
}
253+
// digest keys: X-TIKA:digest:<alg> -> tk:digest:<alg> (algorithm unchanged; MD5 drives
254+
// embedded-doc matching). names() is a snapshot, so remapping while iterating is safe.
255+
for (String name : m.names()) {
256+
if (name.startsWith(LEGACY_DIGEST_PREFIX)) {
257+
remapLegacyKey(m, name, TikaCoreProperties.TIKA_META_PREFIX
258+
+ name.substring(TikaCoreProperties.LEGACY_TIKA_META_PREFIX.length()));
262259
}
263-
} finally {
264-
m.setTrusted(false);
265260
}
266261
}
267262

@@ -281,9 +276,9 @@ private static void remapLegacyKey(Metadata m, String legacyKey, String modernKe
281276
} else {
282277
for (int i = 0; i < legacyVals.length; i++) {
283278
if (i == 0) {
284-
m.set(modernKey, legacyVals[i]);
279+
m.setTrusted(modernKey, legacyVals[i]);
285280
} else {
286-
m.add(modernKey, legacyVals[i]);
281+
m.addTrusted(modernKey, legacyVals[i]);
287282
}
288283
}
289284
}

‎tika-parsers/tika-parsers-ml/tika-inference/src/main/java/org/apache/tika/inference/ChunkSerializer.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,7 @@ public static void mergeInto(
9292
}
9393
existing.addAll(newChunks);
9494
// Chunks are Tika-native output; the default field (tk:chunks) is reserved, so write trusted.
95-
boolean wasTrusted = metadata.isTrusted();
96-
metadata.setTrusted(true);
97-
try {
98-
metadata.set(fieldName, toJson(existing));
99-
} finally {
100-
metadata.setTrusted(wasTrusted);
101-
}
95+
metadata.setTrusted(fieldName, toJson(existing));
10296
}
10397

10498
/**

‎tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-pdf-module/src/main/java/org/apache/tika/parser/pdf/AbstractPDF2XHTML.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -590,13 +590,7 @@ void doOCROnCurrentPage(PDPage pdPage, OcrConfig.Strategy ocrStrategy)
590590
String renderChunks = renderMetadata.get(TikaCoreProperties.TIKA_CHUNKS);
591591
if (renderChunks != null && metadata.get(TikaCoreProperties.TIKA_CHUNKS) == null) {
592592
// tk:chunks is reserved; this is Tika propagating its own native output
593-
boolean wasTrusted = metadata.isTrusted();
594-
metadata.setTrusted(true);
595-
try {
596-
metadata.set(TikaCoreProperties.TIKA_CHUNKS, renderChunks);
597-
} finally {
598-
metadata.setTrusted(wasTrusted);
599-
}
593+
metadata.setTrusted(TikaCoreProperties.TIKA_CHUNKS.getName(), renderChunks);
600594
}
601595
}
602596
} catch (IOException e) {

‎tika-pipes/tika-pipes-core/src/main/java/org/apache/tika/pipes/core/server/EmitHandler.java‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -232,18 +232,12 @@ private static String getContainerStacktrace(FetchEmitTuple t, List<Metadata> me
232232

233233
private void injectUserMetadata(Metadata userMetadata, List<Metadata> metadataList) {
234234
Metadata target = metadataList.get(0);
235-
boolean prev = target.isTrusted();
236-
target.setTrusted(true);
237-
try {
238-
for (String n : userMetadata.names()) {
239-
//overwrite whatever was there
240-
target.set(n, null);
241-
for (String val : userMetadata.getValues(n)) {
242-
target.add(n, val);
243-
}
235+
for (String n : userMetadata.names()) {
236+
//overwrite whatever was there
237+
target.setTrusted(n, null);
238+
for (String val : userMetadata.getValues(n)) {
239+
target.addTrusted(n, val);
244240
}
245-
} finally {
246-
target.setTrusted(prev);
247241
}
248242
}
249243

0 commit comments

Comments
 (0)