Skip to content

Commit

Permalink
Provide mechanism to configure XContent parsing constraints (after up…
Browse files Browse the repository at this point in the history 
…date to Jackson 2.15.0 and above) (opensearch-project#7550)

* Provide mechanism to configure XContent parsing constraints (after update to Jackson 2.15.0 and above)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

* Address code review comments

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
  • Loading branch information
@reta @stephen-crawford
reta authored and stephen-crawford committed May 31, 2023
1 parent 50c9d44 commit 7afb87b
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 0 deletions.
5 changes: 5 additions & 0 deletions libs/x-content/src/main/java/org/opensearch/common/xcontent/cbor/CborXContent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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();
}

Expand Down
6 changes: 6 additions & 0 deletions libs/x-content/src/main/java/org/opensearch/common/xcontent/json/JsonXContent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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();
}

Expand Down
5 changes: 5 additions & 0 deletions libs/x-content/src/main/java/org/opensearch/common/xcontent/smile/SmileXContent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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();
}

Expand Down
5 changes: 5 additions & 0 deletions libs/x-content/src/main/java/org/opensearch/common/xcontent/yaml/YamlXContent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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();
}

Expand Down
91 changes: 91 additions & 0 deletions libs/x-content/src/test/java/org/opensearch/common/xcontent/XContentParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -50,20 +55,106 @@
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;
import static org.hamcrest.Matchers.contains;
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;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.internal.matchers.ThrowableMessageMatcher.hasMessage;

public class XContentParserTests extends OpenSearchTestCase {
private static final Map<XContentType, Supplier<String>> 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<XContentType, Supplier<String>> 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()));
}
}

public void testFloat() throws IOException {
final XContentType xContentType = randomFrom(XContentType.values());
Expand Down

0 comments on commit 7afb87b

Please sign in to comment.