Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 16, 2023
Merged
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
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
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
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
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
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)
andrross marked this conversation as resolved.
Show resolved Hide resolved
);

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