Skip to content

Commit 8f28732

Browse files
imotovimotov
committed
Add proper longitude validation in geo_polygon_query (#30497)
Fixes longitude validation in geo_polygon_query builder. The queries with wrong longitude currently fail but only later during polygon with quite complicated error message. Fixes #30488
1 parent 4474a49 commit 8f28732

File tree

7 files changed

+69
-71
lines changed

7 files changed

+69
-71
lines changed

‎server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
177177
throw new QueryShardException(context, "illegal latitude value [{}] for [{}]", point.lat(),
178178
GeoPolygonQueryBuilder.NAME);
179179
}
180-
if (!GeoUtils.isValidLongitude(point.lat())) {
180+
if (!GeoUtils.isValidLongitude(point.lon())) {
181181
throw new QueryShardException(context, "illegal longitude value [{}] for [{}]", point.lon(),
182182
GeoPolygonQueryBuilder.NAME);
183183
}

‎server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,4 +254,38 @@ public void testIgnoreUnmapped() throws IOException {
254254
QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext()));
255255
assertThat(e.getMessage(), containsString("failed to find geo_point field [unmapped]"));
256256
}
257+
258+
public void testPointValidation() throws IOException {
259+
assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0);
260+
QueryShardContext context = createShardContext();
261+
String queryInvalidLat = "{\n" +
262+
" \"geo_polygon\":{\n" +
263+
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
264+
" \"points\":[\n" +
265+
" [-70, 140],\n" +
266+
" [-80, 30],\n" +
267+
" [-90, 20]\n" +
268+
" ]\n" +
269+
" }\n" +
270+
" }\n" +
271+
"}\n";
272+
273+
QueryShardException e1 = expectThrows(QueryShardException.class, () -> parseQuery(queryInvalidLat).toQuery(context));
274+
assertThat(e1.getMessage(), containsString("illegal latitude value [140.0] for [geo_polygon]"));
275+
276+
String queryInvalidLon = "{\n" +
277+
" \"geo_polygon\":{\n" +
278+
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
279+
" \"points\":[\n" +
280+
" [-70, 40],\n" +
281+
" [-80, 30],\n" +
282+
" [-190, 20]\n" +
283+
" ]\n" +
284+
" }\n" +
285+
" }\n" +
286+
"}\n";
287+
288+
QueryShardException e2 = expectThrows(QueryShardException.class, () -> parseQuery(queryInvalidLon).toQuery(context));
289+
assertThat(e2.getMessage(), containsString("illegal longitude value [-190.0] for [geo_polygon]"));
290+
}
257291
}

‎server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_1.json‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
{
2-
"filtered": {
3-
"query": {
4-
"match_all": {}
5-
},
6-
"filter": {
7-
"geo_polygon": {
8-
"location": {
9-
"points": {
10-
"points": [
11-
[-70, 40],
12-
[-80, 30],
13-
[-90, 20]
14-
]
15-
}
16-
}
2+
"geo_polygon": {
3+
"location": {
4+
"points": {
5+
"points": [
6+
[-70, 40],
7+
[-80, 30],
8+
[-90, 20]
9+
]
1710
}
1811
}
1912
}

‎server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_2.json‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,13 @@
11
{
2-
"filtered": {
3-
"query": {
4-
"match_all": {}
5-
},
6-
"filter": {
7-
"geo_polygon": {
8-
"location": {
9-
"points": [
10-
[-70, 40],
11-
[-80, 30],
12-
[-90, 20]
13-
],
14-
"something_else": {
2+
"geo_polygon": {
3+
"location": {
4+
"points": [
5+
[-70, 40],
6+
[-80, 30],
7+
[-90, 20]
8+
],
9+
"something_else": {
1510

16-
}
17-
18-
}
1911
}
2012
}
2113
}

‎server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_3.json‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
{
2-
"filtered": {
3-
"query": {
4-
"match_all": {}
5-
},
6-
"filter": {
7-
"geo_polygon": {
8-
"location": ["WRONG"]
9-
}
10-
}
2+
"geo_polygon": {
3+
"location": ["WRONG"]
114
}
125
}

‎server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_4.json‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
{
2-
"filtered": {
3-
"query": {
4-
"match_all": {}
2+
"geo_polygon": {
3+
"location": {
4+
"points": [
5+
[-70, 40],
6+
[-80, 30],
7+
[-90, 20]
8+
]
59
},
6-
"filter": {
7-
"geo_polygon": {
8-
"location": {
9-
"points": [
10-
[-70, 40],
11-
[-80, 30],
12-
[-90, 20]
13-
]
14-
},
15-
"bla": true
16-
}
17-
}
10+
"bla": true
1811
}
1912
}

‎server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_5.json‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
{
2-
"filtered": {
3-
"query": {
4-
"match_all": {}
2+
"geo_polygon": {
3+
"location": {
4+
"points": [
5+
[-70, 40],
6+
[-80, 30],
7+
[-90, 20]
8+
]
59
},
6-
"filter": {
7-
"geo_polygon": {
8-
"location": {
9-
"points": [
10-
[-70, 40],
11-
[-80, 30],
12-
[-90, 20]
13-
]
14-
},
15-
"bla": ["array"]
16-
}
17-
}
10+
"bla": ["array"]
1811
}
1912
}

0 commit comments

Comments
 (0)