Fix coerce validation_method in GeoBoundingBoxQueryBuilder (#31747)

The Rectangle constructor validates bounds before coerce has a chance to normalize coordinates so it cannot be used as intermittent storage. This commit removes the Rectangle as an intermittent storage for the bounding box coordinates. Fixes #31718
elastic · Jul 3, 2018 · 396c578 · 396c578
1 parent 1d11407
commit 396c578
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java
@@ -389,7 +389,8 @@ public static GeoBoundingBoxQueryBuilder fromXContent(XContentParser parser) thr
         GeoValidationMethod validationMethod = null;
         boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
 
-        Rectangle bbox = null;
+        // bottom (minLat), top (maxLat), left (minLon), right (maxLon)
+        double[] bbox = null;
         String type = "memory";
 
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
@@ -424,8 +425,8 @@ public static GeoBoundingBoxQueryBuilder fromXContent(XContentParser parser) thr
             throw new ElasticsearchParseException("failed to parse [{}] query. bounding box not provided", NAME);
         }
 
-        final GeoPoint topLeft = new GeoPoint(bbox.maxLat, bbox.minLon);  //just keep the object
-        final GeoPoint bottomRight = new GeoPoint(bbox.minLat, bbox.maxLon);
+        final GeoPoint topLeft = new GeoPoint(bbox[1], bbox[2]);
+        final GeoPoint bottomRight = new GeoPoint(bbox[0], bbox[3]);
 
         GeoBoundingBoxQueryBuilder builder = new GeoBoundingBoxQueryBuilder(fieldName);
         builder.setCorners(topLeft, bottomRight);
@@ -460,7 +461,10 @@ public String getWriteableName() {
         return NAME;
     }
 
-    public static Rectangle parseBoundingBox(XContentParser parser) throws IOException, ElasticsearchParseException {
+    /**
+     * Parses the bounding box and returns bottom, top, left, right coordinates
+     */
+    public static double[] parseBoundingBox(XContentParser parser) throws IOException, ElasticsearchParseException {
         XContentParser.Token token = parser.currentToken();
         if (token != XContentParser.Token.START_OBJECT) {
             throw new ElasticsearchParseException("failed to parse bounding box. Expected start object but found [{}]", token);
@@ -521,8 +525,8 @@ public static Rectangle parseBoundingBox(XContentParser parser) throws IOExcepti
                     + "using well-known text and explicit corners.");
             }
             org.locationtech.spatial4j.shape.Rectangle r = envelope.build();
-            return new Rectangle(r.getMinY(), r.getMaxY(), r.getMinX(), r.getMaxX());
+            return new double[]{r.getMinY(), r.getMaxY(), r.getMinX(), r.getMaxX()};
         }
-        return new Rectangle(bottom, top, left, right);
+        return new double[]{bottom, top, left, right};
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java
@@ -509,6 +509,26 @@ public void testMalformedGeohashes() {
         assertThat(e1.getMessage(), containsString("Conflicting definition found using well-known text and explicit corners."));
     }
 
+    public void testHonorsCoercion() throws IOException {
+        assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
+        String query = "{\n" +
+            "  \"geo_bounding_box\": {\n" +
+            "    \"validation_method\": \"COERCE\",\n" +
+            "    \"" + GEO_POINT_FIELD_NAME + "\": {\n" +
+            "      \"top_left\": {\n" +
+            "        \"lat\": -15.5,\n" +
+            "        \"lon\": 176.5\n" +
+            "      },\n" +
+            "      \"bottom_right\": {\n" +
+            "        \"lat\": -19.6,\n" +
+            "        \"lon\": 181\n" +
+            "      }\n" +
+            "    }\n" +
+            "  }\n" +
+            "}\n";
+        assertGeoBoundingBoxQuery(query);
+    }
+
     @Override
     public void testMustRewrite() throws IOException {
         assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);