Skip to content

Commit

Permalink
ignore_malformed parameter on ip_range data_type throws mapper_parsin…
Browse files Browse the repository at this point in the history 
…g_exception (#2429)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
  • Loading branch information
@reta
reta authored Mar 21, 2022
1 parent 7a6311e commit 1a3dbb7
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 34 deletions.
101 changes: 75 additions & 26 deletions server/src/main/java/org/opensearch/index/mapper/RangeFieldMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,36 @@ public static class Builder extends ParametrizedFieldMapper.Builder {

private final RangeType type;
private final Version indexCreatedVersion;
private final boolean ignoreMalformedByDefault;
private final Parameter<Boolean> ignoreMalformed;

public Builder(String name, RangeType type, Settings settings) {
this(name, type, COERCE_SETTING.get(settings), hasIndexCreated(settings) ? Version.indexCreated(settings) : null);
this(
name,
type,
COERCE_SETTING.get(settings),
IGNORE_MALFORMED_SETTING.get(settings),
hasIndexCreated(settings) ? Version.indexCreated(settings) : null
);
}

public Builder(String name, RangeType type, boolean coerceByDefault, Version indexCreatedVersion) {
this(name, type, coerceByDefault, false /* ignoreMalformedByDefault */, indexCreatedVersion);
}

public Builder(
String name,
RangeType type,
boolean coerceByDefault,
boolean ignoreMalformedByDefault,
Version indexCreatedVersion
) {
super(name);
this.type = type;
this.coerce = Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault);
this.indexCreatedVersion = indexCreatedVersion;
this.ignoreMalformedByDefault = ignoreMalformedByDefault;
this.ignoreMalformed = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault);
if (this.type != RangeType.DATE) {
format.neverSerialize();
locale.neverSerialize();
Expand All @@ -145,7 +165,7 @@ Builder format(String format) {

@Override
protected List<Parameter<?>> getParameters() {
return Arrays.asList(index, hasDocValues, store, coerce, format, locale, boost, meta);
return Arrays.asList(index, hasDocValues, store, coerce, format, locale, boost, meta, ignoreMalformed);
}

protected RangeFieldType setupFieldType(BuilderContext context) {
Expand Down Expand Up @@ -378,6 +398,8 @@ public Query rangeQuery(

private final boolean coerceByDefault;
private final Version indexCreatedVersion;
private final boolean ignoreMalformed;
private final boolean ignoreMalformedByDefault;

private RangeFieldMapper(
String simpleName,
Expand All @@ -397,6 +419,8 @@ private RangeFieldMapper(
this.locale = builder.locale.getValue();
this.coerceByDefault = builder.coerce.getDefaultValue().value();
this.indexCreatedVersion = builder.indexCreatedVersion;
this.ignoreMalformed = builder.ignoreMalformed.getValue();
this.ignoreMalformedByDefault = builder.ignoreMalformedByDefault;
}

boolean coerce() {
Expand All @@ -405,7 +429,7 @@ boolean coerce() {

@Override
public ParametrizedFieldMapper.Builder getMergeBuilder() {
return new Builder(simpleName(), type, coerceByDefault, indexCreatedVersion).init(this);
return new Builder(simpleName(), type, coerceByDefault, ignoreMalformedByDefault, indexCreatedVersion).init(this);
}

@Override
Expand Down Expand Up @@ -442,40 +466,65 @@ protected void parseCreateField(ParseContext context) throws IOException {
boolean includeFrom = DEFAULT_INCLUDE_LOWER;
boolean includeTo = DEFAULT_INCLUDE_UPPER;
XContentParser.Token token;
boolean rangeIsMalformed = false;
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
if (token == XContentParser.Token.FIELD_NAME) {
fieldName = parser.currentName();
} else {
if (fieldName.equals(GT_FIELD.getPreferredName())) {
includeFrom = false;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
}
} else if (fieldName.equals(GTE_FIELD.getPreferredName())) {
includeFrom = true;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
try {
if (fieldName.equals(GT_FIELD.getPreferredName())) {
includeFrom = false;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
}
} else if (fieldName.equals(GTE_FIELD.getPreferredName())) {
includeFrom = true;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
}
} else if (fieldName.equals(LT_FIELD.getPreferredName())) {
includeTo = false;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
}
} else if (fieldName.equals(LTE_FIELD.getPreferredName())) {
includeTo = true;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
}
} else {
throw new MapperParsingException(
"error parsing field [" + name() + "], with unknown parameter [" + fieldName + "]"
);
}
} else if (fieldName.equals(LT_FIELD.getPreferredName())) {
includeTo = false;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
} catch (final IllegalArgumentException e) {
// We have to consume the JSON object in full
if (ignoreMalformed) {
rangeIsMalformed = true;
} else {
throw e;
}
} else if (fieldName.equals(LTE_FIELD.getPreferredName())) {
includeTo = true;
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
}
} else {
throw new MapperParsingException(
"error parsing field [" + name() + "], with unknown parameter [" + fieldName + "]"
);
}
}
}

if (rangeIsMalformed) {
context.addIgnoredField(fieldType().name());
return;
}

range = new Range(rangeType, from, to, includeFrom, includeTo);
} else if (fieldType().rangeType == RangeType.IP && start == XContentParser.Token.VALUE_STRING) {
range = parseIpRangeFromCidr(parser);
try {
range = parseIpRangeFromCidr(parser);
} catch (IllegalArgumentException e) {
if (ignoreMalformed) {
context.addIgnoredField(fieldType().name());
return;
} else {
throw e;
}
}
} else {
throw new MapperParsingException(
"error parsing field [" + name() + "], expected an object but got " + parser.currentName()
Expand Down
96 changes: 88 additions & 8 deletions server/src/test/java/org/opensearch/index/mapper/IpRangeFieldMapperTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,21 @@

import org.apache.lucene.index.DocValuesType;
import org.apache.lucene.index.IndexableField;
import org.opensearch.common.CheckedConsumer;
import org.opensearch.common.Strings;
import org.opensearch.common.bytes.BytesReference;
import org.opensearch.common.compress.CompressedXContent;
import org.opensearch.common.network.InetAddresses;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.index.IndexService;
import org.opensearch.index.termvectors.TermVectorsService;
import org.opensearch.test.OpenSearchSingleNodeTestCase;
import org.junit.Before;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

Expand Down Expand Up @@ -76,14 +80,7 @@ public void testStoreCidr() throws Exception {
cases.put("192.168.0.0/16", "192.168.255.255");
cases.put("192.168.0.0/17", "192.168.127.255");
for (final Map.Entry<String, String> entry : cases.entrySet()) {
ParsedDocument doc = mapper.parse(
new SourceToParse(
"test",
"1",
BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", entry.getKey()).endObject()),
XContentType.JSON
)
);
ParsedDocument doc = mapper.parse(source(b -> b.field("field", entry.getKey())));
IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(3, fields.length);
IndexableField dvField = fields[0];
Expand All @@ -97,5 +94,88 @@ public void testStoreCidr() throws Exception {
+ InetAddresses.toAddrString(InetAddresses.forString(entry.getValue()));
assertThat(storedField.stringValue(), containsString(strVal));
}

// Use alternative form to populate the value:
//
// {
// "field": {
// "gte": "192.168.1.10",
// "lte": "192.168.1.15"
// }
// }
final Map<String, String> params = new HashMap<>();
params.put("gte", "192.168.1.1");
params.put("lte", "192.168.1.15");

final ParsedDocument doc = mapper.parse(source(b -> b.field("field", params)));
final IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(3, fields.length);

final IndexableField storedField = fields[2];
assertThat(storedField.stringValue(), containsString("192.168.1.1 : 192.168.1.15"));
}

public void testIgnoreMalformed() throws Exception {
final DocumentMapper mapper = parser.parse(
"type",
new CompressedXContent(
Strings.toString(
XContentFactory.jsonBuilder()
.startObject()
.startObject("type")
.startObject("properties")
.startObject("field")
.field("type", "ip_range")
.endObject()
.endObject()
.endObject()
.endObject()
)
)
);

final ThrowingRunnable runnable = () -> mapper.parse(source(b -> b.field("field", ":1")));
final MapperParsingException e = expectThrows(MapperParsingException.class, runnable);
assertThat(e.getCause().getMessage(), containsString("Expected [ip/prefix] but was [:1]"));

final DocumentMapper mapper2 = parser.parse(
"type",
new CompressedXContent(
Strings.toString(
XContentFactory.jsonBuilder()
.startObject()
.startObject("type")
.startObject("properties")
.startObject("field")
.field("type", "ip_range")
.field("ignore_malformed", true)
.endObject()
.endObject()
.endObject()
.endObject()
)
)
);

ParsedDocument doc = mapper2.parse(source(b -> b.field("field", ":1")));
IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(0, fields.length);
assertArrayEquals(new String[] { "field" }, TermVectorsService.getValues(doc.rootDoc().getFields("_ignored")));

final Map<String, String> params = new HashMap<>();
params.put("gte", "x.x.x.x");
params.put("lte", "192.168.1.15");

doc = mapper2.parse(source(b -> b.field("field", params)));
fields = doc.rootDoc().getFields("field");
assertEquals(0, fields.length);
assertArrayEquals(new String[] { "field" }, TermVectorsService.getValues(doc.rootDoc().getFields("_ignored")));
}

private final SourceToParse source(CheckedConsumer<XContentBuilder, IOException> build) throws IOException {
XContentBuilder builder = JsonXContent.contentBuilder().startObject();
build.accept(builder);
builder.endObject();
return new SourceToParse("test", "1", BytesReference.bytes(builder), XContentType.JSON);
}
}

0 comments on commit 1a3dbb7

Please sign in to comment.