Skip to content

Commit

Permalink
Ensure Jackson default maximums introduced in 2.16.0 do not conflict …
Browse files Browse the repository at this point in the history
…with OpenSearch settings (#11811)

* Ensure Jackson default maximums introduced in 2.16.0 do not conflict with OpenSearch settings

Signed-off-by: Andriy Redko <[email protected]>

* Address code review comments

Signed-off-by: Andriy Redko <[email protected]>

* Address code review comments

Signed-off-by: Andriy Redko <[email protected]>

---------

Signed-off-by: Andriy Redko <[email protected]>
(cherry picked from commit 5c82ab8)
Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
reta committed Jan 12, 2024
1 parent e42f429 commit 8045c60
Show file tree
Hide file tree
Showing 56 changed files with 278 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add deleted doc count in _cat/shards ([#11678](https://github.com/opensearch-project/OpenSearch/pull/11678))
- Capture information for additional query types and aggregation types ([#11582](https://github.com/opensearch-project/OpenSearch/pull/11582))
- Use slice_size == shard_size heuristic in terms aggs for concurrent segment search and properly calculate the doc_count_error ([#11732](https://github.com/opensearch-project/OpenSearch/pull/11732))
- Ensure Jackson default maximums introduced in 2.16.0 do not conflict with OpenSearch settings ([#11890](https://github.com/opensearch-project/OpenSearch/pull/11890))

### Deprecated

Expand Down
4 changes: 2 additions & 2 deletions buildSrc/version.properties
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ bundled_jdk = 21.0.1+12
# optional dependencies
spatial4j = 0.7
jts = 1.15.0
jackson = 2.16.0
jackson_databind = 2.16.0
jackson = 2.16.1
jackson_databind = 2.16.1
snakeyaml = 2.1
icu4j = 70.1
supercsv = 2.4.0
Expand Down
1 change: 0 additions & 1 deletion client/sniffer/licenses/jackson-core-2.16.0.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions client/sniffer/licenses/jackson-core-2.16.1.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9456bb3cdd0f79f91a5f730a1b1bb041a380c91f

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fd441d574a71e7d10a4f73de6609f881d8cdfeec

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
02a16efeb840c45af1e2f31753dfe76795278b73
1 change: 0 additions & 1 deletion libs/core/licenses/jackson-core-2.16.0.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions libs/core/licenses/jackson-core-2.16.1.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9456bb3cdd0f79f91a5f730a1b1bb041a380c91f
1 change: 0 additions & 1 deletion libs/x-content/licenses/jackson-core-2.16.0.jar.sha1

This file was deleted.

1 change: 1 addition & 0 deletions libs/x-content/licenses/jackson-core-2.16.1.jar.sha1
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9456bb3cdd0f79f91a5f730a1b1bb041a380c91f

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1be7098dccc079171464dca7e386bd8df623b031

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
c4ddbc5277670f2e56b1f5e44e83afa748bcb125

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
8e4f1923d73cd55f2b4c0d56ee4ed80419297354
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/

package org.opensearch.common.xcontent;

import com.fasterxml.jackson.core.StreamReadConstraints;

import org.opensearch.common.annotation.InternalApi;

/**
* Consolidates the XContent constraints (primarily reflecting Jackson's {@link StreamReadConstraints} constraints)
*
* @opensearch.internal
*/
@InternalApi
public interface XContentContraints {
final String DEFAULT_MAX_STRING_LEN_PROPERTY = "opensearch.xcontent.string.length.max";
final String DEFAULT_MAX_NAME_LEN_PROPERTY = "opensearch.xcontent.name.length.max";
final String DEFAULT_MAX_DEPTH_PROPERTY = "opensearch.xcontent.depth.max";

final int DEFAULT_MAX_STRING_LEN = Integer.parseInt(
System.getProperty(DEFAULT_MAX_STRING_LEN_PROPERTY, Integer.toString(Integer.MAX_VALUE) /* no limit */ )
);

final int DEFAULT_MAX_NAME_LEN = Integer.parseInt(
System.getProperty(DEFAULT_MAX_NAME_LEN_PROPERTY, Integer.toString(Integer.MAX_VALUE) /* no limit */ )
);

final int DEFAULT_MAX_DEPTH = Integer.parseInt(
System.getProperty(DEFAULT_MAX_DEPTH_PROPERTY, Integer.toString(Integer.MAX_VALUE) /* no limit */ )
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.StreamReadFeature;
import com.fasterxml.jackson.core.StreamWriteConstraints;
import com.fasterxml.jackson.core.StreamWriteFeature;
import com.fasterxml.jackson.dataformat.cbor.CBORFactory;

import org.opensearch.common.xcontent.XContentContraints;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.xcontent.DeprecationHandler;
import org.opensearch.core.xcontent.MediaType;
Expand All @@ -59,11 +61,7 @@
/**
* 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", Integer.toString(Integer.MAX_VALUE) /* no limit */)
);

public class CborXContent implements XContent, XContentContraints {
public static final boolean USE_FAST_DOUBLE_WRITER = Boolean.getBoolean("opensearch.xcontent.use_fast_double_writer");

public static XContentBuilder contentBuilder() throws IOException {
Expand All @@ -79,7 +77,14 @@ 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());
cborFactory.setStreamWriteConstraints(StreamWriteConstraints.builder().maxNestingDepth(DEFAULT_MAX_DEPTH).build());
cborFactory.setStreamReadConstraints(
StreamReadConstraints.builder()
.maxStringLength(DEFAULT_MAX_STRING_LEN)
.maxNameLength(DEFAULT_MAX_NAME_LEN)
.maxNestingDepth(DEFAULT_MAX_DEPTH)
.build()
);
cborFactory.configure(StreamReadFeature.USE_FAST_DOUBLE_PARSER.mappedFeature(), true);
cborFactory.configure(StreamWriteFeature.USE_FAST_DOUBLE_WRITER.mappedFeature(), USE_FAST_DOUBLE_WRITER);
cborXContent = new CborXContent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.StreamReadFeature;
import com.fasterxml.jackson.core.StreamWriteConstraints;
import com.fasterxml.jackson.core.StreamWriteFeature;

import org.opensearch.common.xcontent.XContentContraints;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.xcontent.DeprecationHandler;
import org.opensearch.core.xcontent.MediaType;
Expand All @@ -58,11 +60,7 @@
/**
* 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", Integer.toString(Integer.MAX_VALUE) /* no limit */)
);

public class JsonXContent implements XContent, XContentContraints {
public static final boolean USE_FAST_DOUBLE_WRITER = Boolean.getBoolean("opensearch.xcontent.use_fast_double_writer");

public static XContentBuilder contentBuilder() throws IOException {
Expand All @@ -81,7 +79,14 @@ 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());
jsonFactory.setStreamWriteConstraints(StreamWriteConstraints.builder().maxNestingDepth(DEFAULT_MAX_DEPTH).build());
jsonFactory.setStreamReadConstraints(
StreamReadConstraints.builder()
.maxStringLength(DEFAULT_MAX_STRING_LEN)
.maxNameLength(DEFAULT_MAX_NAME_LEN)
.maxNestingDepth(DEFAULT_MAX_DEPTH)
.build()
);
jsonFactory.configure(StreamReadFeature.USE_FAST_DOUBLE_PARSER.mappedFeature(), true);
jsonFactory.configure(StreamWriteFeature.USE_FAST_DOUBLE_WRITER.mappedFeature(), USE_FAST_DOUBLE_WRITER);
jsonXContent = new JsonXContent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.StreamReadFeature;
import com.fasterxml.jackson.core.StreamWriteConstraints;
import com.fasterxml.jackson.core.StreamWriteFeature;
import com.fasterxml.jackson.dataformat.smile.SmileFactory;
import com.fasterxml.jackson.dataformat.smile.SmileGenerator;

import org.opensearch.common.xcontent.XContentContraints;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.xcontent.DeprecationHandler;
import org.opensearch.core.xcontent.MediaType;
Expand All @@ -59,11 +61,7 @@
/**
* 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", Integer.toString(Integer.MAX_VALUE) /* no limit */)
);

public class SmileXContent implements XContent, XContentContraints {
public static final boolean USE_FAST_DOUBLE_WRITER = Boolean.getBoolean("opensearch.xcontent.use_fast_double_writer");

public static XContentBuilder contentBuilder() throws IOException {
Expand All @@ -81,7 +79,14 @@ 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());
smileFactory.setStreamWriteConstraints(StreamWriteConstraints.builder().maxNestingDepth(DEFAULT_MAX_DEPTH).build());
smileFactory.setStreamReadConstraints(
StreamReadConstraints.builder()
.maxStringLength(DEFAULT_MAX_STRING_LEN)
.maxNameLength(DEFAULT_MAX_NAME_LEN)
.maxNestingDepth(DEFAULT_MAX_DEPTH)
.build()
);
smileFactory.configure(StreamReadFeature.USE_FAST_DOUBLE_PARSER.mappedFeature(), true);
smileFactory.configure(StreamWriteFeature.USE_FAST_DOUBLE_WRITER.mappedFeature(), USE_FAST_DOUBLE_WRITER);
smileXContent = new SmileXContent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.core.StreamReadFeature;
import com.fasterxml.jackson.core.StreamWriteConstraints;
import com.fasterxml.jackson.core.StreamWriteFeature;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import org.opensearch.common.xcontent.XContentContraints;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.core.xcontent.DeprecationHandler;
import org.opensearch.core.xcontent.MediaType;
Expand All @@ -57,11 +59,7 @@
/**
* 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", Integer.toString(Integer.MAX_VALUE) /* no limit */)
);

public class YamlXContent implements XContent, XContentContraints {
public static final boolean USE_FAST_DOUBLE_WRITER = Boolean.getBoolean("opensearch.xcontent.use_fast_double_writer");

public static XContentBuilder contentBuilder() throws IOException {
Expand All @@ -74,7 +72,14 @@ 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());
yamlFactory.setStreamWriteConstraints(StreamWriteConstraints.builder().maxNestingDepth(DEFAULT_MAX_DEPTH).build());
yamlFactory.setStreamReadConstraints(
StreamReadConstraints.builder()
.maxStringLength(DEFAULT_MAX_STRING_LEN)
.maxNameLength(DEFAULT_MAX_NAME_LEN)
.maxNestingDepth(DEFAULT_MAX_DEPTH)
.build()
);
yamlFactory.configure(StreamReadFeature.USE_FAST_DOUBLE_PARSER.mappedFeature(), true);
yamlFactory.configure(StreamWriteFeature.USE_FAST_DOUBLE_WRITER.mappedFeature(), USE_FAST_DOUBLE_WRITER);
yamlXContent = new YamlXContent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import org.opensearch.common.xcontent.cbor.CborXContent;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.common.xcontent.smile.SmileXContent;
import org.opensearch.common.xcontent.yaml.YamlXContent;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParseException;
Expand Down Expand Up @@ -81,6 +82,28 @@ public class XContentParserTests extends OpenSearchTestCase {
() -> randomAlphaOfLengthBetween(1, 3140000)
);

private static final Map<XContentType, Supplier<String>> FIELD_NAME_GENERATORS = Map.of(
XContentType.JSON,
() -> randomAlphaOfLengthBetween(1, JsonXContent.DEFAULT_MAX_NAME_LEN / 10), /* 10th of limit, OOM otherwise */
XContentType.CBOR,
() -> randomAlphaOfLengthBetween(1, CborXContent.DEFAULT_MAX_NAME_LEN / 10), /* 10th of limit, OOM otherwise */
XContentType.SMILE,
() -> randomAlphaOfLengthBetween(1, SmileXContent.DEFAULT_MAX_NAME_LEN / 10), /* 10th of limit, OOM otherwise */
XContentType.YAML,
() -> randomAlphaOfLengthBetween(1, YamlXContent.DEFAULT_MAX_NAME_LEN / 10) /* 10th of limit, OOM otherwise */
);

private static final Map<XContentType, Supplier<Integer>> DEPTH_GENERATORS = Map.of(
XContentType.JSON,
() -> randomIntBetween(1, Math.min(JsonXContent.DEFAULT_MAX_DEPTH, 1000)), /* limit to 1000, OOM otherwise */
XContentType.CBOR,
() -> randomIntBetween(1, Math.min(CborXContent.DEFAULT_MAX_DEPTH, 1000)), /* limit to 1000, OOM otherwise */
XContentType.SMILE,
() -> randomIntBetween(1, Math.min(SmileXContent.DEFAULT_MAX_DEPTH, 1000)), /* limit to 1000, OOM otherwise */
XContentType.YAML,
() -> randomIntBetween(1, Math.min(YamlXContent.DEFAULT_MAX_DEPTH, 1000)) /* limit to 1000, OOM otherwise */
);

public void testStringOffLimit() throws IOException {
final String field = randomAlphaOfLengthBetween(1, 5);
final String value = randomRealisticUnicodeOfCodepointLength(3145730);
Expand Down Expand Up @@ -136,6 +159,80 @@ public void testString() throws IOException {
}
}

public void testFieldName() throws IOException {
final XContentType xContentType = randomFrom(XContentType.values());

final String field = FIELD_NAME_GENERATORS.get(xContentType).get();
final String value = randomAlphaOfLengthBetween(1, 5);

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());
assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
assertNull(parser.nextToken());
}
}
}

public void testDepth() throws IOException {
final XContentType xContentType = randomFrom(XContentType.values());

final String field = randomAlphaOfLengthBetween(1, 5);
final String value = randomAlphaOfLengthBetween(1, 5);

try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) {
final int maxDepth = DEPTH_GENERATORS.get(xContentType).get() - 1;

for (int depth = 0; depth < maxDepth; ++depth) {
builder.startObject();
builder.field(field + depth);
}

builder.startObject();
if (randomBoolean()) {
builder.field(field, value);
} else {
builder.field(field).value(value);
}
builder.endObject();

for (int depth = 0; depth < maxDepth; ++depth) {
builder.endObject();
}

try (XContentParser parser = createParser(xContentType.xContent(), BytesReference.bytes(builder))) {
for (int depth = 0; depth < maxDepth; ++depth) {
assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken());
assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken());
assertEquals(field + depth, parser.currentName());
}

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());
assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());

for (int depth = 0; depth < maxDepth; ++depth) {
assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken());
}

assertNull(parser.nextToken());
}
}
}

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

Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fd441d574a71e7d10a4f73de6609f881d8cdfeec

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
02a16efeb840c45af1e2f31753dfe76795278b73
Loading

0 comments on commit 8045c60

Please sign in to comment.