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 <heemin@amazon.com>
opensearch-project · Oct 25, 2022 · 7485f4a · 7485f4a
1 parent 272f1d1
commit 7485f4a
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 7 deletions.
diff --git 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
+[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
@@ -245,6 +245,7 @@ default boolean isNormalizable(double coord) {
      * @opensearch.internal
      */
     public static class PointParser<P extends ParsedPoint> extends Parser<List<P>> {
+        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<P> 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
@@ -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()));