From 484002da563d3cb3495f9662c83a311e9b470315 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Fri, 12 May 2023 15:20:51 -0400 Subject: [PATCH 1/2] Provide mechanism to configure XContent parsing constraints (after update to Jackson 2.15.0 and above) Signed-off-by: Andriy Redko --- .../common/xcontent/cbor/CborXContent.java | 5 + .../common/xcontent/json/JsonXContent.java | 6 + .../common/xcontent/smile/SmileXContent.java | 5 + .../common/xcontent/yaml/YamlXContent.java | 5 + .../common/xcontent/XContentParserTests.java | 104 ++++++++++++++++++ 5 files changed, 125 insertions(+) diff --git a/libs/x-content/src/main/java/org/opensearch/common/xcontent/cbor/CborXContent.java b/libs/x-content/src/main/java/org/opensearch/common/xcontent/cbor/CborXContent.java index 5ed275ed2fea0..d7df53e7a0cf5 100644 --- a/libs/x-content/src/main/java/org/opensearch/common/xcontent/cbor/CborXContent.java +++ b/libs/x-content/src/main/java/org/opensearch/common/xcontent/cbor/CborXContent.java @@ -35,6 +35,7 @@ import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.StreamReadConstraints; import com.fasterxml.jackson.dataformat.cbor.CBORFactory; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.MediaType; @@ -56,6 +57,9 @@ * A CBOR based content implementation using Jackson. */ public class CborXContent implements XContent { + public static final int DEFAULT_MAX_STRING_LEN = Integer.parseInt( + System.getProperty("opensearch.xcontent.string.length.max", "50000000" /* ~50 Mb */) + ); public static XContentBuilder contentBuilder() throws IOException { return XContentBuilder.builder(cborXContent); @@ -70,6 +74,7 @@ public static XContentBuilder contentBuilder() throws IOException { // Do not automatically close unclosed objects/arrays in com.fasterxml.jackson.dataformat.cbor.CBORGenerator#close() method cborFactory.configure(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT, false); cborFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); + cborFactory.setStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(DEFAULT_MAX_STRING_LEN).build()); cborXContent = new CborXContent(); } diff --git a/libs/x-content/src/main/java/org/opensearch/common/xcontent/json/JsonXContent.java b/libs/x-content/src/main/java/org/opensearch/common/xcontent/json/JsonXContent.java index 1c2051c5ad3c8..8ff8e7730b189 100644 --- a/libs/x-content/src/main/java/org/opensearch/common/xcontent/json/JsonXContent.java +++ b/libs/x-content/src/main/java/org/opensearch/common/xcontent/json/JsonXContent.java @@ -36,6 +36,8 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.StreamReadConstraints; + import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -55,6 +57,9 @@ * A JSON based content implementation using Jackson. */ public class JsonXContent implements XContent { + public static final int DEFAULT_MAX_STRING_LEN = Integer.parseInt( + System.getProperty("opensearch.xcontent.string.length.max", "50000000" /* ~50 Mb */) + ); public static XContentBuilder contentBuilder() throws IOException { return XContentBuilder.builder(jsonXContent); @@ -72,6 +77,7 @@ public static XContentBuilder contentBuilder() throws IOException { // Do not automatically close unclosed objects/arrays in com.fasterxml.jackson.core.json.UTF8JsonGenerator#close() method jsonFactory.configure(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT, false); jsonFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); + jsonFactory.setStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(DEFAULT_MAX_STRING_LEN).build()); jsonXContent = new JsonXContent(); } diff --git a/libs/x-content/src/main/java/org/opensearch/common/xcontent/smile/SmileXContent.java b/libs/x-content/src/main/java/org/opensearch/common/xcontent/smile/SmileXContent.java index a84c1f3c793eb..e0a39df1589a2 100644 --- a/libs/x-content/src/main/java/org/opensearch/common/xcontent/smile/SmileXContent.java +++ b/libs/x-content/src/main/java/org/opensearch/common/xcontent/smile/SmileXContent.java @@ -35,6 +35,7 @@ import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.StreamReadConstraints; import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.fasterxml.jackson.dataformat.smile.SmileGenerator; import org.opensearch.core.xcontent.DeprecationHandler; @@ -56,6 +57,9 @@ * A Smile based content implementation using Jackson. */ public class SmileXContent implements XContent { + public static final int DEFAULT_MAX_STRING_LEN = Integer.parseInt( + System.getProperty("opensearch.xcontent.string.length.max", "50000000" /* ~50 Mb */) + ); public static XContentBuilder contentBuilder() throws IOException { return XContentBuilder.builder(smileXContent); @@ -72,6 +76,7 @@ public static XContentBuilder contentBuilder() throws IOException { // Do not automatically close unclosed objects/arrays in com.fasterxml.jackson.dataformat.smile.SmileGenerator#close() method smileFactory.configure(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT, false); smileFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); + smileFactory.setStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(DEFAULT_MAX_STRING_LEN).build()); smileXContent = new SmileXContent(); } diff --git a/libs/x-content/src/main/java/org/opensearch/common/xcontent/yaml/YamlXContent.java b/libs/x-content/src/main/java/org/opensearch/common/xcontent/yaml/YamlXContent.java index 250c39429fdbb..1e73cb2bd9c5e 100644 --- a/libs/x-content/src/main/java/org/opensearch/common/xcontent/yaml/YamlXContent.java +++ b/libs/x-content/src/main/java/org/opensearch/common/xcontent/yaml/YamlXContent.java @@ -34,6 +34,7 @@ import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.StreamReadConstraints; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -54,6 +55,9 @@ * A YAML based content implementation using Jackson. */ public class YamlXContent implements XContent { + public static final int DEFAULT_MAX_STRING_LEN = Integer.parseInt( + System.getProperty("opensearch.xcontent.string.length.max", "50000000" /* ~50 Mb */) + ); public static XContentBuilder contentBuilder() throws IOException { return XContentBuilder.builder(yamlXContent); @@ -65,6 +69,7 @@ public static XContentBuilder contentBuilder() throws IOException { static { yamlFactory = new YAMLFactory(); yamlFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); + yamlFactory.setStreamReadConstraints(StreamReadConstraints.builder().maxStringLength(DEFAULT_MAX_STRING_LEN).build()); yamlXContent = new YamlXContent(); } diff --git a/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java b/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java index 81af89a7bc68e..ea179bf881e86 100644 --- a/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java +++ b/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java @@ -33,10 +33,15 @@ package org.opensearch.common.xcontent; import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.core.exc.StreamConstraintsException; +import com.fasterxml.jackson.dataformat.yaml.JacksonYAMLParseException; + import org.opensearch.common.CheckedSupplier; import org.opensearch.common.Strings; import org.opensearch.common.bytes.BytesReference; +import org.opensearch.common.xcontent.cbor.CborXContent; import org.opensearch.common.xcontent.json.JsonXContent; +import org.opensearch.common.xcontent.smile.SmileXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParseException; import org.opensearch.core.xcontent.XContentParser; @@ -50,6 +55,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.function.Supplier; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; @@ -57,6 +63,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasLength; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.in; import static org.hamcrest.Matchers.instanceOf; @@ -64,6 +71,103 @@ import static org.junit.internal.matchers.ThrowableMessageMatcher.hasMessage; public class XContentParserTests extends OpenSearchTestCase { + private static final Map> GENERATORS = Map.of( + XContentType.JSON, + () -> randomAlphaOfLengthBetween(1, JsonXContent.DEFAULT_MAX_STRING_LEN), + XContentType.CBOR, + () -> randomAlphaOfLengthBetween(1, CborXContent.DEFAULT_MAX_STRING_LEN), + XContentType.SMILE, + () -> randomAlphaOfLengthBetween(1, SmileXContent.DEFAULT_MAX_STRING_LEN), + /* YAML parser limitation */ + XContentType.YAML, + () -> randomRealisticUnicodeOfCodepointLengthBetween(1, 3140000) + ); + + private static final Map> OFF_LIMIT_GENERATORS = Map.of( + XContentType.JSON, + () -> randomAlphaOfLength(JsonXContent.DEFAULT_MAX_STRING_LEN + 1), + XContentType.CBOR, + () -> randomAlphaOfLength(CborXContent.DEFAULT_MAX_STRING_LEN + 1), + XContentType.SMILE, + () -> randomAlphaOfLength(SmileXContent.DEFAULT_MAX_STRING_LEN + 1), + /* YAML parser limitation */ + XContentType.YAML, + () -> randomRealisticUnicodeOfCodepointLength(3145730) + ); + + public void testStringOffLimit() throws IOException { + final XContentType xContentType = randomFrom(XContentType.values()); + + final String field = randomAlphaOfLengthBetween(1, 5); + final String value = OFF_LIMIT_GENERATORS.get(xContentType).get(); + + try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) { + builder.startObject(); + if (randomBoolean()) { + builder.field(field, value); + } else { + builder.field(field).value(value); + } + builder.endObject(); + + try (XContentParser parser = createParser(xContentType.xContent(), BytesReference.bytes(builder))) { + assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); + assertEquals(field, parser.currentName()); + assertEquals(XContentParser.Token.VALUE_STRING, parser.nextToken()); + if (xContentType != XContentType.YAML) { + assertThrows(StreamConstraintsException.class, () -> parser.text()); + } else { + assertThrows(JacksonYAMLParseException.class, () -> parser.nextToken()); + } + } + } + } + + public void testString() throws IOException { + final XContentType xContentType = randomFrom(XContentType.values()); + + final String field = randomAlphaOfLengthBetween(1, 5); + final String value = GENERATORS.get(xContentType).get(); + + try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) { + builder.startObject(); + if (randomBoolean()) { + builder.field(field, value); + } else { + builder.field(field).value(value); + } + builder.endObject(); + + final String text; + try (XContentParser parser = createParser(xContentType.xContent(), BytesReference.bytes(builder))) { + assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); + assertEquals(field, parser.currentName()); + assertEquals(XContentParser.Token.VALUE_STRING, parser.nextToken()); + + text = parser.text(); + + assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); + assertNull(parser.nextToken()); + } + + assertThat(text, hasLength(value.length())); + + switch (xContentType) { + case CBOR: + case SMILE: + assertThat(text, instanceOf(String.class)); + break; + case JSON: + case YAML: + assertThat(text, instanceOf(String.class)); + break; + default: + throw new AssertionError("unexpected x-content type [" + xContentType + "]"); + } + } + } public void testFloat() throws IOException { final XContentType xContentType = randomFrom(XContentType.values()); From a82198b80bc135019cf2b4adc0aea04fdf72701e Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Mon, 15 May 2023 08:48:26 -0400 Subject: [PATCH 2/2] Address code review comments Signed-off-by: Andriy Redko --- .../common/xcontent/XContentParserTests.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java b/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java index ea179bf881e86..d14f0da0d331a 100644 --- a/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java +++ b/libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java @@ -153,19 +153,6 @@ public void testString() throws IOException { } assertThat(text, hasLength(value.length())); - - switch (xContentType) { - case CBOR: - case SMILE: - assertThat(text, instanceOf(String.class)); - break; - case JSON: - case YAML: - assertThat(text, instanceOf(String.class)); - break; - default: - throw new AssertionError("unexpected x-content type [" + xContentType + "]"); - } } }