Browse Source

Fix wrong NaN comparison (#61795)

Fixes wrong NaN comparison in error message generator in GeoPolygonDecomposer and PolygonBuilder.

Supersedes #48207

Co-authored-by: Pedro Luiz Cabral Salomon Prado <pedroprado010@users.noreply.github.com>
Igor Motov 5 years ago
parent
commit
286120cdba

+ 2 - 1
server/src/main/java/org/elasticsearch/common/geo/GeoPolygonDecomposer.java

@@ -528,6 +528,7 @@ public class GeoPolygonDecomposer {
                 if (visitedEdge.containsKey(current.coordinate)) {
                     partitionPoint[0] = current.coordinate.getX();
                     partitionPoint[1] = current.coordinate.getY();
+                    partitionPoint[2] = current.coordinate.getZ();
                     if (connectedComponents > 0 && current.next != edge) {
                         throw new InvalidShapeException("Shape contains more than one shared point");
                     }
@@ -576,7 +577,7 @@ public class GeoPolygonDecomposer {
         }
         // First and last coordinates must be equal
         if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) {
-            if (partitionPoint[2] == Double.NaN) {
+            if (Double.isNaN(partitionPoint[2])) {
                 throw new InvalidShapeException("Self-intersection at or near point ["
                     + partitionPoint[0] + "," + partitionPoint[1] + "]");
             } else {

+ 1 - 1
server/src/main/java/org/elasticsearch/common/geo/builders/PolygonBuilder.java

@@ -436,7 +436,7 @@ public class PolygonBuilder extends ShapeBuilder<JtsGeometry, org.elasticsearch.
         }
         // First and last coordinates must be equal
         if (coordinates[0].equals(coordinates[coordinates.length - 1]) == false) {
-            if (partitionPoint[2] == Double.NaN) {
+            if (Double.isNaN(partitionPoint[2])) {
                 throw new InvalidShapeException("Self-intersection at or near point ["
                     + partitionPoint[0] + "," + partitionPoint[1] + "]");
             } else {

+ 21 - 8
server/src/test/java/org/elasticsearch/common/geo/GeometryIndexerTests.java

@@ -19,6 +19,17 @@
 
 package org.elasticsearch.common.geo;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.not;
+
+import java.io.IOException;
+import java.text.ParseException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
 import org.apache.lucene.index.IndexableField;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
@@ -37,14 +48,7 @@ import org.elasticsearch.geometry.Polygon;
 import org.elasticsearch.geometry.Rectangle;
 import org.elasticsearch.index.mapper.GeoShapeIndexer;
 import org.elasticsearch.test.ESTestCase;
-import java.io.IOException;
-import java.text.ParseException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.List;
-
-import static org.hamcrest.Matchers.instanceOf;
+import org.locationtech.spatial4j.exception.InvalidShapeException;
 
 public class GeometryIndexerTests extends ESTestCase {
 
@@ -357,6 +361,15 @@ public class GeometryIndexerTests extends ESTestCase {
             actual(polygon(randomBoolean() ? null : randomBoolean(), 20, 0, 20, 10, -20, 10, -20, 0, 20, 0), randomBoolean()));
     }
 
+    public void testInvalidSelfCrossingPolygon() {
+        Polygon polygon = new Polygon(new LinearRing(
+            new double[]{0, 0, 1, 0.5, 1.5, 1, 2, 2, 0}, new double[]{0, 2, 1.9, 1.8, 1.8, 1.9, 2, 0, 0}
+        ));
+        Exception e = expectThrows(InvalidShapeException.class, () -> indexer.prepareForIndexing(polygon));
+        assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
+        assertThat(e.getMessage(), not(containsString("NaN")));
+    }
+
     private XContentBuilder polygon(Boolean orientation, double... val) throws IOException {
         XContentBuilder pointGeoJson = XContentFactory.jsonBuilder().startObject();
         {

+ 4 - 0
server/src/test/java/org/elasticsearch/common/geo/ShapeBuilderTests.java

@@ -43,6 +43,8 @@ import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertM
 import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertMultiPolygon;
 import static org.elasticsearch.test.hamcrest.ElasticsearchGeoAssertions.assertPolygon;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+
 /**
  * Tests for {@link ShapeBuilder}
  */
@@ -775,8 +777,10 @@ public class ShapeBuilderTests extends ESTestCase {
         );
         Exception e = expectThrows(InvalidShapeException.class, () -> builder.close().buildS4J());
         assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
+        assertThat(e.getMessage(), not(containsString("NaN")));
         e = expectThrows(InvalidShapeException.class, () -> buildGeometry(builder.close()));
         assertThat(e.getMessage(), containsString("Self-intersection at or near point ["));
+        assertThat(e.getMessage(), not(containsString("NaN")));
     }
 
     public Object buildGeometry(ShapeBuilder<?, ?, ?> builder) {