Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 62 additions & 4 deletions tika-core/src/main/java/org/apache/tika/metadata/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
import java.util.Properties;
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;
Expand All @@ -47,6 +50,7 @@ public class Metadata
implements CreativeCommons, Geographic, HttpHeaders, Message, ClimateForcast, TIFF,
TikaMimeKeys, Serializable {

private static final Logger LOG = LoggerFactory.getLogger(Metadata.class);

private static final MetadataWriteLimiter ACCEPT_ALL = new MetadataWriteLimiter() {
@Override
Expand Down Expand Up @@ -304,9 +308,55 @@ private String[] _getValues(final String name) {
* @param value the metadata value.
*/
public void add(final String name, final String value) {
if (blockReservedKeyWrite(name)) {
return;
}
addUnchecked(name, value);
}

/** Trusted add, bypassing the reserved-key guard; used by the Property overloads. TIKA-4769. */
private void addUnchecked(final String name, final String value) {
writeLimiter.add(name, value, metadata);
}

/**
* TIKA-4769: drop a String write to a reserved {@code X-TIKA:} key so untrusted file content
* can't overwrite control-plane metadata. Reserved keys are writable only via their Property.
*/
private static boolean blockReservedKeyWrite(String name) {
if (name != null && name.startsWith(TikaCoreProperties.TIKA_META_PREFIX)) {
LOG.debug("Dropping String write to reserved metadata key '{}'; use its Property "
+ "(TIKA-4769).", name);
return true;
}
return false;
}

/**
* Trusted write for reconstruction paths (clone/merge/deserialize): a reserved {@code X-TIKA:}
* key goes through its Property so the guard doesn't drop it; others by name. TIKA-4769.
*
* @param append add rather than set
*/
public void reconstruct(String name, String value, boolean append) {
if (name != null && name.startsWith(TikaCoreProperties.TIKA_META_PREFIX)) {
Property property = Property.get(name);
if (property != null) {
if (append) {
add(property, value);
} else {
set(property, value);
}
return;
}
}
if (append) {
add(name, value);
} else {
set(name, value);
}
}

/**
* Add a metadata name/value mapping. Add the specified value to the list of
* values associated to the specified metadata name.
Expand All @@ -320,7 +370,7 @@ protected void add(final String name, final String[] newValues) {
set(name, newValues);
} else {
for (String val : newValues) {
add(name, val);
addUnchecked(name, val);
}
}
}
Expand Down Expand Up @@ -351,7 +401,7 @@ public void add(final Property property, final String value) {
set(property, value);
} else {
if (property.isMultiValuePermitted()) {
add(property.getName(), value);
addUnchecked(property.getName(), value);
} else {
throw new PropertyTypeException(
property.getName() + " : " + property.getPropertyType());
Expand Down Expand Up @@ -384,6 +434,14 @@ public void setAll(Properties properties) {
* @param value the metadata value, or <code>null</code>
*/
public void set(String name, String value) {
if (blockReservedKeyWrite(name)) {
return;
}
setUnchecked(name, value);
}

/** Trusted set, bypassing the reserved-key guard. TIKA-4769. */
private void setUnchecked(String name, String value) {
writeLimiter.set(name, value, metadata);
}

Expand All @@ -393,7 +451,7 @@ protected void set(String name, String[] values) {
if (values != null) {
metadata.remove(name);
for (String v : values) {
add(name, v);
addUnchecked(name, v);
}
} else {
metadata.remove(name);
Expand All @@ -419,7 +477,7 @@ public void set(Property property, String value) {
}
}
} else {
set(property.getName(), value);
setUnchecked(property.getName(), value);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ protected static Metadata mergeMetadata(Metadata newMetadata, Metadata lastMetad
if (newVals == null || newVals.length == 0) {
// Metadata only in previous run, keep old values
for (String val : oldVals) {
newMetadata.add(n, val);
newMetadata.reconstruct(n, val, true);
}
} else if (Arrays.deepEquals(oldVals, newVals)) {
// Metadata is the same, nothing to do
Expand All @@ -136,7 +136,7 @@ protected static Metadata mergeMetadata(Metadata newMetadata, Metadata lastMetad
// Use the earlier value(s) in place of this/these one/s
newMetadata.remove(n);
for (String val : oldVals) {
newMetadata.add(n, val);
newMetadata.reconstruct(n, val, true);
}
continue;
case LAST_WINS:
Expand All @@ -147,11 +147,11 @@ protected static Metadata mergeMetadata(Metadata newMetadata, Metadata lastMetad
List<String> vals = new ArrayList<>(Arrays.asList(oldVals));
newMetadata.remove(n);
for (String oldVal : oldVals) {
newMetadata.add(n, oldVal);
newMetadata.reconstruct(n, oldVal, true);
}
for (String newVal : newVals) {
if (!vals.contains(newVal)) {
newMetadata.add(n, newVal);
newMetadata.reconstruct(n, newVal, true);
vals.add(newVal);
}
}
Expand Down Expand Up @@ -318,7 +318,7 @@ private void parse(TikaInputStream tis, ContentHandler handler,
for (String n : metadata.names()) {
originalMetadata.remove(n);
for (String val : metadata.getValues(n)) {
originalMetadata.add(n, val);
originalMetadata.reconstruct(n, val, true);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,10 @@ public static Metadata cloneMetadata(Metadata m) {

for (String n : m.names()) {
if (!m.isMultiValued(n)) {
clone.set(n, m.get(n));
clone.reconstruct(n, m.get(n), false);
} else {
String[] vals = m.getValues(n);
for (String val : vals) {
clone.add(n, val);
for (String val : m.getValues(n)) {
clone.reconstruct(n, val, true);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
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 org.junit.jupiter.api.Test;

/** TIKA-4769: reserved {@code X-TIKA:} keys can't be overwritten by String writes, only via Property. */
public class MetadataInternalKeyGuardTest {

@Test
public void testStringWriteToInternalKeyIsDropped() {
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");
assertNull(metadata.get(TikaCoreProperties.TIKA_CONTENT.getName()));
}

@Test
public void testStringAddToInternalMultiValueKeyIsDropped() {
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");
}

@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");
assertEquals("trusted", metadata.get(TikaCoreProperties.TIKA_CONTENT));
}

@Test
public void testPropertyWriteToInternalKeyStillWorks() {
Metadata metadata = new Metadata();
metadata.set(TikaCoreProperties.TIKA_CONTENT, "legit");
assertEquals("legit", metadata.get(TikaCoreProperties.TIKA_CONTENT));

metadata.add(TikaCoreProperties.TIKA_PARSED_BY, "p1");
metadata.add(TikaCoreProperties.TIKA_PARSED_BY, "p2");
assertArrayEquals(new String[] {"p1", "p2"},
metadata.getValues(TikaCoreProperties.TIKA_PARSED_BY));
}

@Test
public void testNonReservedStringKeysAreUnaffected() {
Metadata metadata = new Metadata();
// unregistered key passes through
metadata.set("my:customKey", "value");
assertEquals("value", metadata.get("my:customKey"));

// a registered non-reserved property (dc:description) written by name still works
metadata.set(TikaCoreProperties.DESCRIPTION.getName(), "hello");
assertEquals("hello", metadata.get(TikaCoreProperties.DESCRIPTION));
}

@Test
public void testNullNameDoesNotThrow() {
Metadata metadata = new Metadata();
// no NPE on a null name
metadata.set((String) null, "x");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ private boolean addField(JsonParser jsonParser, Metadata metadata) throws IOExce
}

if (token.isScalarValue()) {
metadata.set(field, jsonParser.getText());
metadata.reconstruct(field, jsonParser.getText(), false);
} else if (jsonParser.isExpectedStartArrayToken()) {
token = jsonParser.nextToken();
while (token != null) {
if (token == JsonToken.END_ARRAY) {
return true;
} else if (token.isScalarValue()) {
metadata.add(field, jsonParser.getText());
metadata.reconstruct(field, jsonParser.getText(), true);
} else {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
package org.apache.tika.serialization;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -130,4 +131,25 @@ public void testLargeValues() throws Exception {
Metadata deserialized = JsonMetadata.fromJson(new StringReader(writer.toString()));
assertEquals(m, deserialized);
}

@Test
public void testInternalKeysRoundTrip() throws Exception {
//TIKA-4769: reserved keys must survive serialize->deserialize (reconstructed via Property).
Metadata m = new Metadata();
m.set(TikaCoreProperties.TIKA_CONTENT, "the content");
m.add(TikaCoreProperties.TIKA_PARSED_BY, "org.apache.tika.parser.DefaultParser");
m.add(TikaCoreProperties.TIKA_PARSED_BY, "org.apache.tika.parser.pdf.PDFParser");
m.add("k1", "v1");

StringWriter writer = new StringWriter();
JsonMetadata.toJson(m, writer);
Metadata deserialized = JsonMetadata.fromJson(new StringReader(writer.toString()));

assertEquals("the content", deserialized.get(TikaCoreProperties.TIKA_CONTENT));
assertArrayEquals(
new String[] {"org.apache.tika.parser.DefaultParser",
"org.apache.tika.parser.pdf.PDFParser"},
deserialized.getValues(TikaCoreProperties.TIKA_PARSED_BY));
assertEquals(m, deserialized);
}
}
Loading