Fix a bug on handling an invalid array value for point type field (op…

…ensearch-project#4900) With this commit, appropriate exception is thrown when an array of four values are provided for point type field. Signed-off-by: Heemin Kim <[email protected]> Signed-off-by: Heemin Kim <[email protected]>
rursprung · Oct 25, 2022 · 0f477a2 · 0f477a2
1 parent 2268af2
commit 0f477a2
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -165,6 +165,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))

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()));