From 8f28732487ac5229803a777fdbe4c54ff4dd0a9e Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Thu, 10 May 2018 11:14:08 -0400 Subject: [PATCH] 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 --- .../index/query/GeoPolygonQueryBuilder.java | 2 +- .../query/GeoPolygonQueryBuilderTests.java | 34 +++++++++++++++++++ .../index/query/geo_polygon_exception_1.json | 23 +++++-------- .../index/query/geo_polygon_exception_2.json | 24 +++++-------- .../index/query/geo_polygon_exception_3.json | 11 ++---- .../index/query/geo_polygon_exception_4.json | 23 +++++-------- .../index/query/geo_polygon_exception_5.json | 23 +++++-------- 7 files changed, 69 insertions(+), 71 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java index 34c29ab0f1890..a07b4186ed594 100644 --- a/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java @@ -177,7 +177,7 @@ protected Query doToQuery(QueryShardContext context) throws IOException { throw new QueryShardException(context, "illegal latitude value [{}] for [{}]", point.lat(), GeoPolygonQueryBuilder.NAME); } - if (!GeoUtils.isValidLongitude(point.lat())) { + if (!GeoUtils.isValidLongitude(point.lon())) { throw new QueryShardException(context, "illegal longitude value [{}] for [{}]", point.lon(), GeoPolygonQueryBuilder.NAME); } diff --git a/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java index b5fb281454010..4ca37638a2226 100644 --- a/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java @@ -254,4 +254,38 @@ public void testIgnoreUnmapped() throws IOException { QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); assertThat(e.getMessage(), containsString("failed to find geo_point field [unmapped]")); } + + public void testPointValidation() throws IOException { + assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length > 0); + QueryShardContext context = createShardContext(); + String queryInvalidLat = "{\n" + + " \"geo_polygon\":{\n" + + " \"" + GEO_POINT_FIELD_NAME + "\":{\n" + + " \"points\":[\n" + + " [-70, 140],\n" + + " [-80, 30],\n" + + " [-90, 20]\n" + + " ]\n" + + " }\n" + + " }\n" + + "}\n"; + + QueryShardException e1 = expectThrows(QueryShardException.class, () -> parseQuery(queryInvalidLat).toQuery(context)); + assertThat(e1.getMessage(), containsString("illegal latitude value [140.0] for [geo_polygon]")); + + String queryInvalidLon = "{\n" + + " \"geo_polygon\":{\n" + + " \"" + GEO_POINT_FIELD_NAME + "\":{\n" + + " \"points\":[\n" + + " [-70, 40],\n" + + " [-80, 30],\n" + + " [-190, 20]\n" + + " ]\n" + + " }\n" + + " }\n" + + "}\n"; + + QueryShardException e2 = expectThrows(QueryShardException.class, () -> parseQuery(queryInvalidLon).toQuery(context)); + assertThat(e2.getMessage(), containsString("illegal longitude value [-190.0] for [geo_polygon]")); + } } diff --git a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_1.json b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_1.json index e079d64eb8fda..94b9fae143a25 100644 --- a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_1.json +++ b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_1.json @@ -1,19 +1,12 @@ { - "filtered": { - "query": { - "match_all": {} - }, - "filter": { - "geo_polygon": { - "location": { - "points": { - "points": [ - [-70, 40], - [-80, 30], - [-90, 20] - ] - } - } + "geo_polygon": { + "location": { + "points": { + "points": [ + [-70, 40], + [-80, 30], + [-90, 20] + ] } } } diff --git a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_2.json b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_2.json index 0955c260727df..a7363452c54bb 100644 --- a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_2.json +++ b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_2.json @@ -1,21 +1,13 @@ { - "filtered": { - "query": { - "match_all": {} - }, - "filter": { - "geo_polygon": { - "location": { - "points": [ - [-70, 40], - [-80, 30], - [-90, 20] - ], - "something_else": { + "geo_polygon": { + "location": { + "points": [ + [-70, 40], + [-80, 30], + [-90, 20] + ], + "something_else": { - } - - } } } } diff --git a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_3.json b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_3.json index 0ac2a7bbb3abc..eef8c1ca074d1 100644 --- a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_3.json +++ b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_3.json @@ -1,12 +1,5 @@ { - "filtered": { - "query": { - "match_all": {} - }, - "filter": { - "geo_polygon": { - "location": ["WRONG"] - } - } + "geo_polygon": { + "location": ["WRONG"] } } diff --git a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_4.json b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_4.json index 51f6ad0037ea6..b2a65825c36f6 100644 --- a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_4.json +++ b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_4.json @@ -1,19 +1,12 @@ { - "filtered": { - "query": { - "match_all": {} + "geo_polygon": { + "location": { + "points": [ + [-70, 40], + [-80, 30], + [-90, 20] + ] }, - "filter": { - "geo_polygon": { - "location": { - "points": [ - [-70, 40], - [-80, 30], - [-90, 20] - ] - }, - "bla": true - } - } + "bla": true } } diff --git a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_5.json b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_5.json index 6f058f551cf60..5287154af42cc 100644 --- a/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_5.json +++ b/server/src/test/resources/org/elasticsearch/index/query/geo_polygon_exception_5.json @@ -1,19 +1,12 @@ { - "filtered": { - "query": { - "match_all": {} + "geo_polygon": { + "location": { + "points": [ + [-70, 40], + [-80, 30], + [-90, 20] + ] }, - "filter": { - "geo_polygon": { - "location": { - "points": [ - [-70, 40], - [-80, 30], - [-90, 20] - ] - }, - "bla": ["array"] - } - } + "bla": ["array"] } }