Skip to content

Commit d52cd9d

Browse files
authored
ignore_malformed parameter on ip_range data_type throws mapper_parsing_exception (#2429) (#2542)
Signed-off-by: Andriy Redko <[email protected]>
1 parent ff679bf commit d52cd9d

File tree

2 files changed

+163
-35
lines changed

2 files changed

+163
-35
lines changed

server/src/main/java/org/opensearch/index/mapper/RangeFieldMapper.java

+75-26
Original file line numberDiff line numberDiff line change
@@ -118,16 +118,36 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
118118

119119
private final RangeType type;
120120
private final Version indexCreatedVersion;
121+
private final boolean ignoreMalformedByDefault;
122+
private final Parameter<Boolean> ignoreMalformed;
121123

122124
public Builder(String name, RangeType type, Settings settings) {
123-
this(name, type, COERCE_SETTING.get(settings), hasIndexCreated(settings) ? Version.indexCreated(settings) : null);
125+
this(
126+
name,
127+
type,
128+
COERCE_SETTING.get(settings),
129+
IGNORE_MALFORMED_SETTING.get(settings),
130+
hasIndexCreated(settings) ? Version.indexCreated(settings) : null
131+
);
124132
}
125133

126134
public Builder(String name, RangeType type, boolean coerceByDefault, Version indexCreatedVersion) {
135+
this(name, type, coerceByDefault, false /* ignoreMalformedByDefault */, indexCreatedVersion);
136+
}
137+
138+
public Builder(
139+
String name,
140+
RangeType type,
141+
boolean coerceByDefault,
142+
boolean ignoreMalformedByDefault,
143+
Version indexCreatedVersion
144+
) {
127145
super(name);
128146
this.type = type;
129147
this.coerce = Parameter.explicitBoolParam("coerce", true, m -> toType(m).coerce, coerceByDefault);
130148
this.indexCreatedVersion = indexCreatedVersion;
149+
this.ignoreMalformedByDefault = ignoreMalformedByDefault;
150+
this.ignoreMalformed = Parameter.boolParam("ignore_malformed", true, m -> toType(m).ignoreMalformed, ignoreMalformedByDefault);
131151
if (this.type != RangeType.DATE) {
132152
format.neverSerialize();
133153
locale.neverSerialize();
@@ -145,7 +165,7 @@ Builder format(String format) {
145165

146166
@Override
147167
protected List<Parameter<?>> getParameters() {
148-
return Arrays.asList(index, hasDocValues, store, coerce, format, locale, boost, meta);
168+
return Arrays.asList(index, hasDocValues, store, coerce, format, locale, boost, meta, ignoreMalformed);
149169
}
150170

151171
protected RangeFieldType setupFieldType(BuilderContext context) {
@@ -378,6 +398,8 @@ public Query rangeQuery(
378398

379399
private final boolean coerceByDefault;
380400
private final Version indexCreatedVersion;
401+
private final boolean ignoreMalformed;
402+
private final boolean ignoreMalformedByDefault;
381403

382404
private RangeFieldMapper(
383405
String simpleName,
@@ -397,6 +419,8 @@ private RangeFieldMapper(
397419
this.locale = builder.locale.getValue();
398420
this.coerceByDefault = builder.coerce.getDefaultValue().value();
399421
this.indexCreatedVersion = builder.indexCreatedVersion;
422+
this.ignoreMalformed = builder.ignoreMalformed.getValue();
423+
this.ignoreMalformedByDefault = builder.ignoreMalformedByDefault;
400424
}
401425

402426
boolean coerce() {
@@ -405,7 +429,7 @@ boolean coerce() {
405429

406430
@Override
407431
public ParametrizedFieldMapper.Builder getMergeBuilder() {
408-
return new Builder(simpleName(), type, coerceByDefault, indexCreatedVersion).init(this);
432+
return new Builder(simpleName(), type, coerceByDefault, ignoreMalformedByDefault, indexCreatedVersion).init(this);
409433
}
410434

411435
@Override
@@ -442,40 +466,65 @@ protected void parseCreateField(ParseContext context) throws IOException {
442466
boolean includeFrom = DEFAULT_INCLUDE_LOWER;
443467
boolean includeTo = DEFAULT_INCLUDE_UPPER;
444468
XContentParser.Token token;
469+
boolean rangeIsMalformed = false;
445470
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
446471
if (token == XContentParser.Token.FIELD_NAME) {
447472
fieldName = parser.currentName();
448473
} else {
449-
if (fieldName.equals(GT_FIELD.getPreferredName())) {
450-
includeFrom = false;
451-
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
452-
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
453-
}
454-
} else if (fieldName.equals(GTE_FIELD.getPreferredName())) {
455-
includeFrom = true;
456-
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
457-
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
474+
try {
475+
if (fieldName.equals(GT_FIELD.getPreferredName())) {
476+
includeFrom = false;
477+
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
478+
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
479+
}
480+
} else if (fieldName.equals(GTE_FIELD.getPreferredName())) {
481+
includeFrom = true;
482+
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
483+
from = rangeType.parseFrom(fieldType, parser, coerce.value(), includeFrom);
484+
}
485+
} else if (fieldName.equals(LT_FIELD.getPreferredName())) {
486+
includeTo = false;
487+
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
488+
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
489+
}
490+
} else if (fieldName.equals(LTE_FIELD.getPreferredName())) {
491+
includeTo = true;
492+
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
493+
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
494+
}
495+
} else {
496+
throw new MapperParsingException(
497+
"error parsing field [" + name() + "], with unknown parameter [" + fieldName + "]"
498+
);
458499
}
459-
} else if (fieldName.equals(LT_FIELD.getPreferredName())) {
460-
includeTo = false;
461-
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
462-
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
500+
} catch (final IllegalArgumentException e) {
501+
// We have to consume the JSON object in full
502+
if (ignoreMalformed) {
503+
rangeIsMalformed = true;
504+
} else {
505+
throw e;
463506
}
464-
} else if (fieldName.equals(LTE_FIELD.getPreferredName())) {
465-
includeTo = true;
466-
if (parser.currentToken() != XContentParser.Token.VALUE_NULL) {
467-
to = rangeType.parseTo(fieldType, parser, coerce.value(), includeTo);
468-
}
469-
} else {
470-
throw new MapperParsingException(
471-
"error parsing field [" + name() + "], with unknown parameter [" + fieldName + "]"
472-
);
473507
}
474508
}
475509
}
510+
511+
if (rangeIsMalformed) {
512+
context.addIgnoredField(fieldType().name());
513+
return;
514+
}
515+
476516
range = new Range(rangeType, from, to, includeFrom, includeTo);
477517
} else if (fieldType().rangeType == RangeType.IP && start == XContentParser.Token.VALUE_STRING) {
478-
range = parseIpRangeFromCidr(parser);
518+
try {
519+
range = parseIpRangeFromCidr(parser);
520+
} catch (IllegalArgumentException e) {
521+
if (ignoreMalformed) {
522+
context.addIgnoredField(fieldType().name());
523+
return;
524+
} else {
525+
throw e;
526+
}
527+
}
479528
} else {
480529
throw new MapperParsingException(
481530
"error parsing field [" + name() + "], expected an object but got " + parser.currentName()

server/src/test/java/org/opensearch/index/mapper/IpRangeFieldMapperTests.java

+88-9
Original file line numberDiff line numberDiff line change
@@ -33,17 +33,21 @@
3333

3434
import org.apache.lucene.index.DocValuesType;
3535
import org.apache.lucene.index.IndexableField;
36+
import org.opensearch.common.CheckedConsumer;
3637
import org.opensearch.common.Strings;
3738
import org.opensearch.common.bytes.BytesReference;
3839
import org.opensearch.common.compress.CompressedXContent;
3940
import org.opensearch.common.network.InetAddresses;
4041
import org.opensearch.common.xcontent.XContentBuilder;
4142
import org.opensearch.common.xcontent.XContentFactory;
4243
import org.opensearch.common.xcontent.XContentType;
44+
import org.opensearch.common.xcontent.json.JsonXContent;
4345
import org.opensearch.index.IndexService;
46+
import org.opensearch.index.termvectors.TermVectorsService;
4447
import org.opensearch.test.OpenSearchSingleNodeTestCase;
4548
import org.junit.Before;
4649

50+
import java.io.IOException;
4751
import java.util.HashMap;
4852
import java.util.Map;
4953

@@ -76,15 +80,7 @@ public void testStoreCidr() throws Exception {
7680
cases.put("192.168.0.0/16", "192.168.255.255");
7781
cases.put("192.168.0.0/17", "192.168.127.255");
7882
for (final Map.Entry<String, String> entry : cases.entrySet()) {
79-
ParsedDocument doc = mapper.parse(
80-
new SourceToParse(
81-
"test",
82-
"type",
83-
"1",
84-
BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("field", entry.getKey()).endObject()),
85-
XContentType.JSON
86-
)
87-
);
83+
ParsedDocument doc = mapper.parse(source(b -> b.field("field", entry.getKey())));
8884
IndexableField[] fields = doc.rootDoc().getFields("field");
8985
assertEquals(3, fields.length);
9086
IndexableField dvField = fields[0];
@@ -98,5 +94,88 @@ public void testStoreCidr() throws Exception {
9894
+ InetAddresses.toAddrString(InetAddresses.forString(entry.getValue()));
9995
assertThat(storedField.stringValue(), containsString(strVal));
10096
}
97+
98+
// Use alternative form to populate the value:
99+
//
100+
// {
101+
// "field": {
102+
// "gte": "192.168.1.10",
103+
// "lte": "192.168.1.15"
104+
// }
105+
// }
106+
final Map<String, String> params = new HashMap<>();
107+
params.put("gte", "192.168.1.1");
108+
params.put("lte", "192.168.1.15");
109+
110+
final ParsedDocument doc = mapper.parse(source(b -> b.field("field", params)));
111+
final IndexableField[] fields = doc.rootDoc().getFields("field");
112+
assertEquals(3, fields.length);
113+
114+
final IndexableField storedField = fields[2];
115+
assertThat(storedField.stringValue(), containsString("192.168.1.1 : 192.168.1.15"));
116+
}
117+
118+
public void testIgnoreMalformed() throws Exception {
119+
final DocumentMapper mapper = parser.parse(
120+
"type",
121+
new CompressedXContent(
122+
Strings.toString(
123+
XContentFactory.jsonBuilder()
124+
.startObject()
125+
.startObject("type")
126+
.startObject("properties")
127+
.startObject("field")
128+
.field("type", "ip_range")
129+
.endObject()
130+
.endObject()
131+
.endObject()
132+
.endObject()
133+
)
134+
)
135+
);
136+
137+
final ThrowingRunnable runnable = () -> mapper.parse(source(b -> b.field("field", ":1")));
138+
final MapperParsingException e = expectThrows(MapperParsingException.class, runnable);
139+
assertThat(e.getCause().getMessage(), containsString("Expected [ip/prefix] but was [:1]"));
140+
141+
final DocumentMapper mapper2 = parser.parse(
142+
"type",
143+
new CompressedXContent(
144+
Strings.toString(
145+
XContentFactory.jsonBuilder()
146+
.startObject()
147+
.startObject("type")
148+
.startObject("properties")
149+
.startObject("field")
150+
.field("type", "ip_range")
151+
.field("ignore_malformed", true)
152+
.endObject()
153+
.endObject()
154+
.endObject()
155+
.endObject()
156+
)
157+
)
158+
);
159+
160+
ParsedDocument doc = mapper2.parse(source(b -> b.field("field", ":1")));
161+
IndexableField[] fields = doc.rootDoc().getFields("field");
162+
assertEquals(0, fields.length);
163+
assertArrayEquals(new String[] { "field" }, TermVectorsService.getValues(doc.rootDoc().getFields("_ignored")));
164+
165+
final Map<String, String> params = new HashMap<>();
166+
params.put("gte", "x.x.x.x");
167+
params.put("lte", "192.168.1.15");
168+
169+
doc = mapper2.parse(source(b -> b.field("field", params)));
170+
fields = doc.rootDoc().getFields("field");
171+
assertEquals(0, fields.length);
172+
assertArrayEquals(new String[] { "field" }, TermVectorsService.getValues(doc.rootDoc().getFields("_ignored")));
173+
}
174+
175+
private final SourceToParse source(CheckedConsumer<XContentBuilder, IOException> build) throws IOException {
176+
XContentBuilder builder = JsonXContent.contentBuilder().startObject();
177+
build.accept(builder);
178+
builder.endObject();
179+
return new SourceToParse("test", "_doc", "1", BytesReference.bytes(builder), XContentType.JSON);
101180
}
102181
}

0 commit comments

Comments
 (0)