From c07a4abee1baab491b6f37ac6c51281ade17eb9f Mon Sep 17 00:00:00 2001
From: Heemin Kim <heemin@amazon.com>
Date: Tue, 25 Oct 2022 16:58:31 +0000
Subject: [PATCH] Fix a bug on handling an invalid array value for point type
 field (#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 <heemin@amazon.com>
(cherry picked from commit 0f477a2d0aae4d75313a7f26407cda8594bd981e)
---
 CHANGELOG.md                                      |  2 ++
 .../mapper/AbstractPointGeometryFieldMapper.java  | 15 +++++++++------
 .../index/mapper/GeoPointFieldMapperTests.java    |  6 ++++++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 37ff3013617b7..c464a3704666d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -71,6 +71,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Fixing Gradle warnings associated with publishPluginZipPublicationToXxx tasks ([#4696](https://github.com/opensearch-project/OpenSearch/pull/4696))
 - Fixed randomly failing test ([4774](https://github.com/opensearch-project/OpenSearch/pull/4774))
 - Fix recovery path for searchable snapshots ([4813](https://github.com/opensearch-project/OpenSearch/pull/4813))
+- 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
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<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
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()));