From 7485f4ab6426364dbd5ab2ae755a21bfad6bce44 Mon Sep 17 00:00:00 2001 From: Heemin Kim Date: Mon, 24 Oct 2022 18:21:10 -0700 Subject: [PATCH] Fix a bug on handling an invalid array value for point type field With this commit, appropriate exception is thrown when an array of four values are provided for point type field. Signed-off-by: Heemin Kim --- CHANGELOG.md | 3 ++- .../mapper/AbstractPointGeometryFieldMapper.java | 15 +++++++++------ .../index/mapper/GeoPointFieldMapperTests.java | 6 ++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e1743df0fe90..8eda120b17f71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -163,6 +163,7 @@ Inspired from [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)) - [BUG]: flaky test index/80_geo_point/Single point test([#4860](https://github.com/opensearch-project/OpenSearch/pull/4860)) - Fix bug in SlicedInputStream with zero length ([#4863](https://github.com/opensearch-project/OpenSearch/pull/4863)) +- Fix a bug on handling an invalid array value for point type field #4900([#4900](https://github.com/opensearch-project/OpenSearch/pull/4900)) ### Security - CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341)) @@ -189,4 +190,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Security [Unreleased]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...HEAD -[2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...2.x \ No newline at end of file +[2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.2.0...2.x diff --git a/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java index 305351ca63b9a..b5db3546185ba 100644 --- a/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/AbstractPointGeometryFieldMapper.java @@ -245,6 +245,7 @@ default boolean isNormalizable(double coord) { * @opensearch.internal */ public static class PointParser

extends Parser> { + private static final int MAX_NUMBER_OF_VALUES_IN_ARRAY_FORMAT = 3; /** * Note that this parser is only used for formatting values. */ @@ -287,7 +288,7 @@ public List

parse(XContentParser parser) throws IOException, ParseException { if (parser.currentToken() == XContentParser.Token.START_ARRAY) { parser.nextToken(); if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { - XContentBuilder xContentBuilder = reConstructArrayXContent(parser); + XContentBuilder xContentBuilder = reconstructArrayXContent(parser); try ( XContentParser subParser = createParser( parser.getXContentRegistry(), @@ -328,18 +329,20 @@ private XContentParser createParser( return subParser; } - private XContentBuilder reConstructArrayXContent(XContentParser parser) throws IOException { + private XContentBuilder reconstructArrayXContent(XContentParser parser) throws IOException { XContentBuilder builder = XContentFactory.jsonBuilder().startArray(); - int count = 0; + int numberOfValuesAdded = 0; while (parser.currentToken() != XContentParser.Token.END_ARRAY) { - if (++count > 3) { - break; - } if (parser.currentToken() != XContentParser.Token.VALUE_NUMBER) { throw new OpenSearchParseException("numeric value expected"); } builder.value(parser.doubleValue()); parser.nextToken(); + + // Allows one more value to be added so that the error case can be handled by a parser + if (++numberOfValuesAdded > MAX_NUMBER_OF_VALUES_IN_ARRAY_FORMAT) { + break; + } } builder.endArray(); return builder; diff --git a/server/src/test/java/org/opensearch/index/mapper/GeoPointFieldMapperTests.java b/server/src/test/java/org/opensearch/index/mapper/GeoPointFieldMapperTests.java index 1b4c95d9ceb8f..060901a3eba38 100644 --- a/server/src/test/java/org/opensearch/index/mapper/GeoPointFieldMapperTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/GeoPointFieldMapperTests.java @@ -164,6 +164,12 @@ public void testLatLonInOneValueArray() throws Exception { assertThat(doc.rootDoc().getFields("field"), arrayWithSize(4)); } + public void testLatLonInArrayMoreThanThreeValues() throws Exception { + DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_point").field("ignore_z_value", true))); + Exception e = expectThrows(MapperParsingException.class, () -> mapper.parse(source(b -> b.array("field", 1.2, 1.3, 1.4, 1.5)))); + assertThat(e.getCause().getMessage(), containsString("[geo_point] field type does not accept more than 3 values")); + } + public void testLonLatArray() throws Exception { DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); ParsedDocument doc = mapper.parse(source(b -> b.startArray("field").value(1.3).value(1.2).endArray()));