This is an automated email from the ASF dual-hosted git repository. tballison pushed a commit to branch TIKA-4816-metadata-key-api in repository https://gitbox.apache.org/repos/asf/tika.git
commit 7672ca16fab86025c0c520189b845dc5a95faf6d Author: tallison <[email protected]> AuthorDate: Wed Aug 12 15:53:00 2026 -0400 TIKA-4816 stage 7: flip reserved-key guard from silent-drop to throw blockReservedKeyWrite becomes checkNotReserved: Metadata#set/add(String, String) now throw IllegalArgumentException on reserved tk:/X-TIKA: names. With document-derived writers migrated off the String route (previous commit, including the ESReporterConfig load-time guard and test-route conversions that keep this flip's blast radius zero in-repo), the throw is unreachable from file input; it fires only on internal misuse (fail fast) or crafted files hitting unmigrated third-party parsers. Guard tests convert from drop- to throw-assertions; reconstruct/putAll/ trusted-route contracts unchanged. Full reactor green. Gated on the regression-corpus run before merge. Co-Authored-By: Claude Fable 5 <[email protected]> --- .../java/org/apache/tika/metadata/Metadata.java | 32 ++++++------- .../metadata/MetadataInternalKeyGuardTest.java | 55 +++++++++++++--------- 2 files changed, 48 insertions(+), 39 deletions(-) diff --git a/tika-core/src/main/java/org/apache/tika/metadata/Metadata.java b/tika-core/src/main/java/org/apache/tika/metadata/Metadata.java index 2a6dd56a1e..064766c5b8 100644 --- a/tika-core/src/main/java/org/apache/tika/metadata/Metadata.java +++ b/tika-core/src/main/java/org/apache/tika/metadata/Metadata.java @@ -32,9 +32,6 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.TimeZone; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.tika.metadata.Property.PropertyType; import org.apache.tika.metadata.writefilter.MetadataWriteLimiter; import org.apache.tika.metadata.writefilter.MetadataWriteLimiterFactory; @@ -48,8 +45,6 @@ public class Metadata implements CreativeCommons, Geographic, HttpHeaders, Message, ClimateForcast, TIFF, Serializable { - private static final Logger LOG = LoggerFactory.getLogger(Metadata.class); - private static final MetadataWriteLimiter ACCEPT_ALL = new MetadataWriteLimiter() { @Override public void add(String field, String value, Map<String, String[]> data) { @@ -304,11 +299,11 @@ public class Metadata * * @param name the metadata name. * @param value the metadata value. + * @throws IllegalArgumentException if {@code name} is a reserved Tika-native + * ({@code tk:}) key; use its {@link Property} or {@link #addTrusted}. */ public void add(final String name, final String value) { - if (blockReservedKeyWrite(name)) { - return; - } + checkNotReserved(name); addTrusted(name, value); } @@ -322,13 +317,18 @@ public class Metadata writeLimiter.add(name, value, metadata); } - /** Drop String writes to reserved Tika-native keys; use their Property or {@link #addTrusted}/{@link #setTrusted(String, String)}. */ - private boolean blockReservedKeyWrite(String name) { + /** + * Reject String writes to reserved Tika-native keys; use their Property or + * {@link #addTrusted}/{@link #setTrusted(String, String)}. + */ + private void checkNotReserved(String name) { if (ReservedNamespaces.isTikaNative(name)) { - LOG.debug("Dropping String write to reserved metadata key '{}'; use its Property.", name); - return true; + throw new IllegalArgumentException( + "Writing reserved key '" + name + "' via the String API is not allowed: " + + "Tika-computed keys are set internally via curated Properties, and " + + "document-derived key names must go through a KeyPrefix; see the " + + "4.x migration guide."); } - return false; } /** @@ -454,11 +454,11 @@ public class Metadata * * @param name the metadata name. * @param value the metadata value, or <code>null</code> + * @throws IllegalArgumentException if {@code name} is a reserved Tika-native + * ({@code tk:}) key; use its {@link Property} or {@link #setTrusted}. */ public void set(String name, String value) { - if (blockReservedKeyWrite(name)) { - return; - } + checkNotReserved(name); setTrusted(name, value); } diff --git a/tika-core/src/test/java/org/apache/tika/metadata/MetadataInternalKeyGuardTest.java b/tika-core/src/test/java/org/apache/tika/metadata/MetadataInternalKeyGuardTest.java index 6bda921d26..8f28ad14cf 100644 --- a/tika-core/src/test/java/org/apache/tika/metadata/MetadataInternalKeyGuardTest.java +++ b/tika-core/src/test/java/org/apache/tika/metadata/MetadataInternalKeyGuardTest.java @@ -19,45 +19,52 @@ package org.apache.tika.metadata; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.Test; -/** Reserved Tika-native ({@code tk:}) keys can't be overwritten by String writes, only via Property. */ +/** Reserved Tika-native ({@code tk:}) keys can't be written by String writes -- only via Property; the String route throws. */ public class MetadataInternalKeyGuardTest { @Test public void testLegacyXTikaPrefixStaysReserved() { Metadata metadata = new Metadata(); // pre-4.0.0 prefix stays reserved so a crafted file can't forge it during the 4.x window - metadata.add(TikaCoreProperties.LEGACY_TIKA_META_PREFIX + "Parsed-By", "org.evil.FakeParser"); - assertNull(metadata.get(TikaCoreProperties.LEGACY_TIKA_META_PREFIX + "Parsed-By"), - "legacy X-TIKA: String write must still be dropped"); + IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, + () -> metadata.add(TikaCoreProperties.LEGACY_TIKA_META_PREFIX + "Parsed-By", "org.evil.FakeParser"), + "legacy X-TIKA: String write must still throw"); + assertTrue(ex.getMessage().contains(TikaCoreProperties.LEGACY_TIKA_META_PREFIX + "Parsed-By")); + assertNull(metadata.get(TikaCoreProperties.LEGACY_TIKA_META_PREFIX + "Parsed-By")); } @Test - public void testStringWriteToInternalKeyIsDropped() { + public void testStringWriteToInternalKeyThrows() { Metadata metadata = new Metadata(); // hostile scrape - metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "injected"); - assertNull(metadata.get(TikaCoreProperties.TIKA_CONTENT), - "String write to an internal key must be dropped"); + assertThrows(IllegalArgumentException.class, + () -> metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "injected"), + "String write to an internal key must throw"); + assertNull(metadata.get(TikaCoreProperties.TIKA_CONTENT)); assertNull(metadata.get(TikaCoreProperties.TIKA_CONTENT.getName())); } @Test - public void testStringAddToInternalMultiValueKeyIsDropped() { + public void testStringAddToInternalMultiValueKeyThrows() { Metadata metadata = new Metadata(); - metadata.add(TikaCoreProperties.TIKA_PARSED_BY.getName(), "org.evil.FakeParser"); - assertArrayEquals(new String[0], metadata.getValues(TikaCoreProperties.TIKA_PARSED_BY), - "String add to an internal key must be dropped"); + assertThrows(IllegalArgumentException.class, + () -> metadata.add(TikaCoreProperties.TIKA_PARSED_BY.getName(), "org.evil.FakeParser"), + "String add to an internal key must throw"); + assertArrayEquals(new String[0], metadata.getValues(TikaCoreProperties.TIKA_PARSED_BY)); } @Test public void testStringWriteCannotOverwriteTrustedInternalValue() { Metadata metadata = new Metadata(); metadata.set(TikaCoreProperties.TIKA_CONTENT, "trusted"); - // String-path attempt must not clobber - metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "injected"); + // String-path attempt must throw, not clobber + assertThrows(IllegalArgumentException.class, + () -> metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "injected")); assertEquals("trusted", metadata.get(TikaCoreProperties.TIKA_CONTENT)); } @@ -110,7 +117,7 @@ public class MetadataInternalKeyGuardTest { String unregistered = TikaCoreProperties.TIKA_META_PREFIX + "noSuchRegisteredProperty"; assertNull(Property.get(unregistered), "precondition: key must be unregistered"); - metadata.set(unregistered, "dropped"); + assertThrows(IllegalArgumentException.class, () -> metadata.set(unregistered, "thrown")); assertNull(metadata.get(unregistered)); metadata.reconstruct(unregistered, "kept", false); @@ -126,8 +133,9 @@ public class MetadataInternalKeyGuardTest { metadata.setTrusted(TikaCoreProperties.TIKA_CONTENT.getName(), "trusted"); assertEquals("trusted", metadata.get(TikaCoreProperties.TIKA_CONTENT)); - // untrusted String-path attempt must not clobber - metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "blocked"); + // untrusted String-path attempt must throw, not clobber + assertThrows(IllegalArgumentException.class, + () -> metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "blocked")); assertEquals("trusted", metadata.get(TikaCoreProperties.TIKA_CONTENT)); } @@ -153,25 +161,26 @@ public class MetadataInternalKeyGuardTest { /** * Design doc "Honest framing": {@code reconstruct} is a deliberately trusted route, * not subject to the String-route guard -- for both a reserved name with a registered - * curated Property and one with none. Contrasts directly against the drop asserted by - * {@link #testStringWriteToInternalKeyIsDropped()} / + * curated Property and one with none. Contrasts directly against the throw asserted by + * {@link #testStringWriteToInternalKeyThrows()} / * {@link #testReconstructPreservesUnregisteredReservedKey()} on the same names, so a - * regression that made {@code reconstruct} start dropping (or the guard start + * regression that made {@code reconstruct} start throwing (or the guard start * exempting it) would be caught here either way. */ @Test - public void testReconstructIsNotSubjectToReservedKeyDrop() { + public void testReconstructIsNotSubjectToReservedKeyGuard() { Metadata metadata = new Metadata(); // registered curated Property - metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "dropped-by-guard"); + assertThrows(IllegalArgumentException.class, + () -> metadata.set(TikaCoreProperties.TIKA_CONTENT.getName(), "thrown-by-guard")); assertNull(metadata.get(TikaCoreProperties.TIKA_CONTENT)); metadata.reconstruct(TikaCoreProperties.TIKA_CONTENT.getName(), "lands-via-reconstruct", false); assertEquals("lands-via-reconstruct", metadata.get(TikaCoreProperties.TIKA_CONTENT)); // reserved but unregistered String unregistered = TikaCoreProperties.TIKA_META_PREFIX + "noSuchRegisteredProperty2"; - metadata.set(unregistered, "dropped-by-guard"); + assertThrows(IllegalArgumentException.class, () -> metadata.set(unregistered, "thrown-by-guard")); assertNull(metadata.get(unregistered)); metadata.reconstruct(unregistered, "lands-via-reconstruct", false); assertEquals("lands-via-reconstruct", metadata.get(unregistered));
