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.

Sign up for GitHub

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

Jump to bottom

Adding depth check in doc parser for deep nested document #5199

Merged
merged 4 commits into from
Feb 28, 2023
Merged

Adding depth check in doc parser for deep nested document #5199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
552afc4
Adding depth check in doc parser for deep nested document
Feb 23, 2023
21407ca
Adding depth check nested array fields
Dec 5, 2022
c78a71e
Addressed code review comment
Jan 4, 2023
c7b3897
Added test for deep nested object and array combination
Jan 5, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
- Fix compression support for h2c protocol ([#4944](https://github.com/opensearch-project/OpenSearch/pull/4944))
- Support OpenSSL Provider with default Netty allocator ([#5460](https://github.com/opensearch-project/OpenSearch/pull/5460))
- Added depth check in doc parser for deep nested document ([#5199](https://github.com/opensearch-project/OpenSearch/pull/5199))

### Security

Expand Down
145 changes: 80 additions & 65 deletions server/src/main/java/org/opensearch/index/mapper/DocumentParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,34 +425,40 @@ private static void innerParseObject(
String currentFieldName,
XContentParser.Token token
) throws IOException {
assert token == XContentParser.Token.FIELD_NAME || token == XContentParser.Token.END_OBJECT;
String[] paths = null;
while (token != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
paths = splitAndValidatePath(currentFieldName);
if (containsDisabledObjectMapper(mapper, paths)) {
parser.nextToken();
parser.skipChildren();
try {
assert token == XContentParser.Token.FIELD_NAME || token == XContentParser.Token.END_OBJECT;
String[] paths = null;
context.incrementFieldCurrentDepth();
context.checkFieldDepthLimit();
while (token != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
currentFieldName = parser.currentName();
paths = splitAndValidatePath(currentFieldName);
if (containsDisabledObjectMapper(mapper, paths)) {
parser.nextToken();
parser.skipChildren();
}
} else if (token == XContentParser.Token.START_OBJECT) {
parseObject(context, mapper, currentFieldName, paths);
} else if (token == XContentParser.Token.START_ARRAY) {
parseArray(context, mapper, currentFieldName, paths);
} else if (token == XContentParser.Token.VALUE_NULL) {
parseNullValue(context, mapper, currentFieldName, paths);
} else if (token == null) {
throw new MapperParsingException(
"object mapping for ["
+ mapper.name()
+ "] tried to parse field ["
+ currentFieldName
+ "] as object, but got EOF, has a concrete value been provided to it?"
);
} else if (token.isValue()) {
parseValue(context, mapper, currentFieldName, token, paths);
}
} else if (token == XContentParser.Token.START_OBJECT) {
parseObject(context, mapper, currentFieldName, paths);
} else if (token == XContentParser.Token.START_ARRAY) {
parseArray(context, mapper, currentFieldName, paths);
} else if (token == XContentParser.Token.VALUE_NULL) {
parseNullValue(context, mapper, currentFieldName, paths);
} else if (token == null) {
throw new MapperParsingException(
"object mapping for ["
+ mapper.name()
+ "] tried to parse field ["
+ currentFieldName
+ "] as object, but got EOF, has a concrete value been provided to it?"
);
} else if (token.isValue()) {
parseValue(context, mapper, currentFieldName, token, paths);
token = parser.nextToken();
}
token = parser.nextToken();
} finally {
context.decrementFieldCurrentDepth();
}
}

Expand Down Expand Up @@ -563,50 +569,59 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,

private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName, String[] paths)
throws IOException {
String arrayFieldName = lastFieldName;

Mapper mapper = getMapper(context, parentMapper, lastFieldName, paths);
if (mapper != null) {
// There is a concrete mapper for this field already. Need to check if the mapper
// expects an array, if so we pass the context straight to the mapper and if not
// we serialize the array components
if (parsesArrayValue(mapper)) {
parseObjectOrField(context, mapper);
} else {
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
}
} else {
arrayFieldName = paths[paths.length - 1];
lastFieldName = arrayFieldName;
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
parentMapper = parentMapperTuple.v2();
ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentMapper, context);
if (dynamic == ObjectMapper.Dynamic.STRICT) {
throw new StrictDynamicMappingException(parentMapper.fullPath(), arrayFieldName);
} else if (dynamic == ObjectMapper.Dynamic.TRUE) {
Mapper.Builder builder = context.root().findTemplateBuilder(context, arrayFieldName, XContentFieldType.OBJECT);
if (builder == null) {
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
try {
String arrayFieldName = lastFieldName;
context.incrementFieldArrayDepth();
context.checkFieldArrayDepthLimit();

Mapper mapper = getMapper(context, parentMapper, lastFieldName, paths);
if (mapper != null) {
// There is a concrete mapper for this field already. Need to check if the mapper
// expects an array, if so we pass the context straight to the mapper and if not
// we serialize the array components
if (parsesArrayValue(mapper)) {
parseObjectOrField(context, mapper);
} else {
Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path());
mapper = builder.build(builderContext);
assert mapper != null;
if (parsesArrayValue(mapper)) {
context.addDynamicMapper(mapper);
context.path().add(arrayFieldName);
parseObjectOrField(context, mapper);
context.path().remove();
} else {
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
}
} else {
arrayFieldName = paths[paths.length - 1];
lastFieldName = arrayFieldName;
Tuple<Integer, ObjectMapper> parentMapperTuple = getDynamicParentMapper(context, paths, parentMapper);
parentMapper = parentMapperTuple.v2();
ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentMapper, context);
if (dynamic == ObjectMapper.Dynamic.STRICT) {
throw new StrictDynamicMappingException(parentMapper.fullPath(), arrayFieldName);
} else if (dynamic == ObjectMapper.Dynamic.TRUE) {
Mapper.Builder builder = context.root().findTemplateBuilder(context, arrayFieldName, XContentFieldType.OBJECT);
if (builder == null) {
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
} else {
Mapper.BuilderContext builderContext = new Mapper.BuilderContext(
context.indexSettings().getSettings(),
context.path()
);
mapper = builder.build(builderContext);
assert mapper != null;
if (parsesArrayValue(mapper)) {
context.addDynamicMapper(mapper);
context.path().add(arrayFieldName);
parseObjectOrField(context, mapper);
context.path().remove();
} else {
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
}
}
} else {
// TODO: shouldn't this skip, not parse?
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
}
for (int i = 0; i < parentMapperTuple.v1(); i++) {
context.path().remove();
}
} else {
// TODO: shouldn't this skip, not parse?
parseNonDynamicArray(context, parentMapper, lastFieldName, arrayFieldName);
}
for (int i = 0; i < parentMapperTuple.v1(); i++) {
context.path().remove();
}
} finally {
context.decrementFieldArrayDepth();
}
}

Expand Down
110 changes: 110 additions & 0 deletions server/src/main/java/org/opensearch/index/mapper/ParseContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.lucene.index.IndexableField;
import org.apache.lucene.util.BytesRef;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.OpenSearchParseException;
import org.opensearch.index.IndexSettings;

import java.util.ArrayList;
Expand Down Expand Up @@ -312,6 +313,36 @@ public void addIgnoredField(String field) {
public Collection<String> getIgnoredFields() {
return in.getIgnoredFields();
}

@Override
public void incrementFieldCurrentDepth() {
in.incrementFieldCurrentDepth();
}

@Override
public void decrementFieldCurrentDepth() {
in.decrementFieldCurrentDepth();
}

@Override
public void checkFieldDepthLimit() {
in.checkFieldDepthLimit();
}

@Override
public void incrementFieldArrayDepth() {
in.incrementFieldArrayDepth();
}

@Override
public void decrementFieldArrayDepth() {
in.decrementFieldArrayDepth();
}

@Override
public void checkFieldArrayDepthLimit() {
in.checkFieldArrayDepthLimit();
}
}

/**
Expand Down Expand Up @@ -345,6 +376,14 @@ public static class InternalParseContext extends ParseContext {

private long numNestedDocs;

private long currentFieldDepth;

private final long maxAllowedFieldDepth;

private long currentArrayDepth;

private final long maxAllowedArrayDepth;

private final List<Mapper> dynamicMappers;

private boolean docsReversed = false;
Expand All @@ -371,6 +410,10 @@ public InternalParseContext(
this.dynamicMappers = new ArrayList<>();
this.maxAllowedNumNestedDocs = indexSettings.getMappingNestedDocsLimit();
this.numNestedDocs = 0L;
this.currentFieldDepth = 0L;
this.currentArrayDepth = 0L;
this.maxAllowedFieldDepth = indexSettings.getMappingDepthLimit();
this.maxAllowedArrayDepth = indexSettings.getMappingDepthLimit();
}

@Override
Expand Down Expand Up @@ -522,6 +565,60 @@ public void addIgnoredField(String field) {
public Collection<String> getIgnoredFields() {
return Collections.unmodifiableCollection(ignoredFields);
}

@Override
public void incrementFieldCurrentDepth() {
this.currentFieldDepth++;
}

@Override
public void decrementFieldCurrentDepth() {
if (this.currentFieldDepth > 0) {
this.currentFieldDepth--;
}
}

@Override
public void checkFieldDepthLimit() {
if (this.currentFieldDepth > maxAllowedFieldDepth) {
this.currentFieldDepth = 0;
throw new OpenSearchParseException(
"The depth of the field has exceeded the allowed limit of ["
+ maxAllowedFieldDepth
+ "]."
+ " This limit can be set by changing the ["
+ MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey()
+ "] index level setting."
);
}
}

@Override
public void incrementFieldArrayDepth() {
this.currentArrayDepth++;
}

@Override
public void decrementFieldArrayDepth() {
if (this.currentArrayDepth > 0) {
this.currentArrayDepth--;
}
}

@Override
public void checkFieldArrayDepthLimit() {
if (this.currentArrayDepth > maxAllowedArrayDepth) {
this.currentArrayDepth = 0;
throw new OpenSearchParseException(
"The depth of the nested array field has exceeded the allowed limit of ["
+ maxAllowedArrayDepth
+ "]."
+ " This limit can be set by changing the ["
+ MapperService.INDEX_MAPPING_DEPTH_LIMIT_SETTING.getKey()
+ "] index level setting."
);
}
}
}

/**
Expand Down Expand Up @@ -687,4 +784,17 @@ public final <T> T parseExternalValue(Class<T> clazz) {
* Get dynamic mappers created while parsing.
*/
public abstract List<Mapper> getDynamicMappers();

public abstract void incrementFieldCurrentDepth();

public abstract void decrementFieldCurrentDepth();

public abstract void checkFieldDepthLimit();

public abstract void incrementFieldArrayDepth();

public abstract void decrementFieldArrayDepth();

public abstract void checkFieldArrayDepthLimit();

}
Loading