Skip to content

Commit 6343c18

Browse files
authored
Fix infinite loop when polygonizing a circle with centre on the pole (#70875) (#70957)
This PR prevents the algorithm to run on circles that contain a pole.
1 parent b9c2e4d commit 6343c18

File tree

4 files changed

+44
-8
lines changed

4 files changed

+44
-8
lines changed

docs/reference/ingest/processors/circle.asciidoc

+2
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ The circle can be represented as either a WKT circle or a GeoJSON circle. The re
5757
polygon will be represented and indexed using the same format as the input circle. WKT will
5858
be translated to a WKT polygon, and GeoJSON circles will be translated to GeoJSON polygons.
5959

60+
IMPORTANT: Circles that contain a pole are not supported.
61+
6062
==== Example: Circle defined in Well Known Text
6163

6264
In this example a circle defined in WKT format is indexed

test/framework/src/main/java/org/elasticsearch/geo/GeometryTestUtils.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,12 @@ public static double randomAlt() {
4646
}
4747

4848
public static Circle randomCircle(boolean hasAlt) {
49+
org.apache.lucene.geo.Circle luceneCircle = GeoTestUtil.nextCircle();
4950
if (hasAlt) {
50-
return new Circle(randomLon(), randomLat(), ESTestCase.randomDouble(),
51-
ESTestCase.randomDoubleBetween(0, 100, false));
51+
return new Circle(luceneCircle.getLon(), luceneCircle.getLat(), ESTestCase.randomDouble(),
52+
luceneCircle.getRadius());
5253
} else {
53-
return new Circle(randomLon(), randomLat(), ESTestCase.randomDoubleBetween(0, 100, false));
54+
return new Circle(luceneCircle.getLon(), luceneCircle.getLat(), luceneCircle.getRadius());
5455
}
5556
}
5657

x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/SpatialUtils.java

+11
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,27 @@ private SpatialUtils() {}
2323
* Makes an n-gon, centered at the provided circle's center, and each vertex approximately
2424
* {@link Circle#getRadiusMeters()} away from the center.
2525
*
26+
* It throws an IllegalArgumentException if the circle contains a pole.
27+
*
2628
* This does not split the polygon across the date-line. Relies on {@link GeoShapeIndexer} to
2729
* split prepare polygon for indexing.
2830
*
2931
* Adapted from from org.apache.lucene.geo.GeoTestUtil
3032
* */
3133
public static Polygon createRegularGeoShapePolygon(Circle circle, int gons) {
34+
if (SloppyMath.haversinMeters(circle.getLat(), circle.getLon(), 90, 0) < circle.getRadiusMeters()) {
35+
throw new IllegalArgumentException("circle [" + circle.toString() + "] contains the north pole. " +
36+
"It cannot be translated to a polygon");
37+
}
38+
if (SloppyMath.haversinMeters(circle.getLat(), circle.getLon(), -90, 0) < circle.getRadiusMeters()) {
39+
throw new IllegalArgumentException("circle [" + circle.toString() + "] contains the south pole. " +
40+
"It cannot be translated to a polygon");
41+
}
3242
double[][] result = new double[2][];
3343
result[0] = new double[gons+1];
3444
result[1] = new double[gons+1];
3545
for(int i=0; i<gons; i++) {
46+
// make sure we do not start at angle 0 or we have issues at the poles
3647
double angle = i * (360.0 / gons);
3748
double x = Math.cos(SloppyMath.toRadians(angle));
3849
double y = Math.sin(SloppyMath.toRadians(angle));

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/SpatialUtilsTests.java

+27-5
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,43 @@
77
package org.elasticsearch.xpack.spatial;
88

99
import org.apache.lucene.util.SloppyMath;
10+
import org.elasticsearch.geo.GeometryTestUtils;
1011
import org.elasticsearch.geometry.Circle;
1112
import org.elasticsearch.geometry.LinearRing;
1213
import org.elasticsearch.geometry.Polygon;
1314
import org.elasticsearch.test.ESTestCase;
1415

1516
import static org.hamcrest.Matchers.closeTo;
17+
import static org.hamcrest.Matchers.containsString;
1618
import static org.hamcrest.Matchers.equalTo;
1719

1820
public class SpatialUtilsTests extends ESTestCase {
1921

2022
public void testCreateRegularGeoShapePolygon() {
21-
double lon = randomDoubleBetween(-20, 20, true);
22-
double lat = randomDoubleBetween(-20, 20, true);
23-
double radiusMeters = randomDoubleBetween(10, 10000, true);
24-
Circle circle = new Circle(lon, lat, radiusMeters);
23+
final Circle circle = randomValueOtherThanMany(
24+
c -> SloppyMath.haversinMeters(c.getLat(), c.getLon(), 90, 0) < c.getRadiusMeters()
25+
|| SloppyMath.haversinMeters(c.getLat(), c.getLon(), -90, 0) < c.getRadiusMeters(),
26+
() -> GeometryTestUtils.randomCircle(true));
27+
doRegularGeoShapePolygon(circle);
28+
}
29+
30+
public void testCircleContainsNorthPole() {
31+
final Circle circle = randomValueOtherThanMany(
32+
c -> SloppyMath.haversinMeters(c.getLat(), c.getLon(), 90, 0) >= c.getRadiusMeters(),
33+
() -> GeometryTestUtils.randomCircle(true));
34+
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> doRegularGeoShapePolygon(circle));
35+
assertThat(ex.getMessage(), containsString("contains the north pole"));
36+
}
37+
38+
public void testCircleContainsSouthPole() {
39+
final Circle circle = randomValueOtherThanMany(
40+
c -> SloppyMath.haversinMeters(c.getLat(), c.getLon(), -90, 0) >= c.getRadiusMeters(),
41+
() -> GeometryTestUtils.randomCircle(true));
42+
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> doRegularGeoShapePolygon(circle));
43+
assertThat(ex.getMessage(), containsString("contains the south pole"));
44+
}
45+
46+
private void doRegularGeoShapePolygon(Circle circle) {
2547
int numSides = randomIntBetween(4, 1000);
2648
Polygon polygon = SpatialUtils.createRegularGeoShapePolygon(circle, numSides);
2749
LinearRing outerShell = polygon.getPolygon();
@@ -35,7 +57,7 @@ public void testCreateRegularGeoShapePolygon() {
3557
for (int i = 0; i < numPoints ; i++) {
3658
double actualDistance = SloppyMath
3759
.haversinMeters(circle.getY(), circle.getX(), outerShell.getY(i), outerShell.getX(i));
38-
assertThat(actualDistance, closeTo(radiusMeters, 0.1));
60+
assertThat(actualDistance, closeTo(circle.getRadiusMeters(), 0.1));
3961
}
4062
}
4163

0 commit comments

Comments
 (0)