honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add stricter geohash parsing (#30376) 

Adds verification that geohashes are not empty and contain only
valid characters. It fixes the issue when en empty geohash is
treated as [-180, -90] and geohashes with non-geohash character
are getting resolved into invalid coordinates.

Closes #23579
This commit is contained in:
Igor Motov 2018-05-07 13:56:39 -04:00 committed by Colin Goodheart-Smithe
parent baf7b92b09
commit 2518fefa37
No known key found for this signature in database
GPG Key ID: F975E7BDD739B3C7
7 changed files with 132 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
docs/CHANGELOG.asciidoc
View File

@ -177,6 +177,21 @@ Machine Learning::
* Account for gaps in data counts after job is reopened ({pull}30294[#30294])
Add validation that geohashes are not empty and don't contain unsupported characters ({pull}30376[#30376])
[[release-notes-6.3.1]]
== Elasticsearch version 6.3.1
//[float]
//=== New Features
//[float]
//=== Enhancements
[float]
=== Bug Fixes
Reduce the number of object allocations made by {security} when resolving the indices and aliases for a request ({pull}30180[#30180])
Rollup::
* Validate timezone in range queries to ensure they match the selected job when
searching ({pull}30338[#30338])

6
server/src/main/java/org/elasticsearch/common/geo/GeoHashUtils.java
View File

@ -171,11 +171,17 @@ public class GeoHashUtils {
* Encode to a morton long value from a given geohash string
*/
public static final long mortonEncode(final String hash) {
if (hash.isEmpty()) {
throw new IllegalArgumentException("empty geohash");
}
int level = 11;
long b;
long l = 0L;
for(char c : hash.toCharArray()) {
b = (long)(BASE_32_STRING.indexOf(c));
if (b < 0) {
throw new IllegalArgumentException("unsupported symbol [" + c + "] in geohash [" + hash + "]");
}
l |= (b<<((level--*5) + MORTON_OFFSET));
if (level < 0) {
// We cannot handle more than 12 levels

8
server/src/main/java/org/elasticsearch/common/geo/GeoPoint.java
View File

@ -28,7 +28,6 @@ import org.apache.lucene.util.BytesRef;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Strings;
import java.io.IOException;
import java.util.Arrays;
@ -126,7 +125,12 @@ public final class GeoPoint implements ToXContentFragment {
}
public GeoPoint resetFromGeoHash(String geohash) {
final long hash = mortonEncode(geohash);
final long hash;
try {
hash = mortonEncode(geohash);
} catch (IllegalArgumentException ex) {
throw new ElasticsearchParseException(ex.getMessage(), ex);
}
return this.reset(GeoHashUtils.decodeLatitude(hash), GeoHashUtils.decodeLongitude(hash));
}

58
server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java
View File

@ -299,14 +299,7 @@ public class GeoPointFieldMapper extends FieldMapper implements ArrayValueMapper
if (token == XContentParser.Token.START_ARRAY) {
// its an array of array of lon/lat [ [1.2, 1.3], [1.4, 1.5] ]
while (token != XContentParser.Token.END_ARRAY) {
try {
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
} catch (ElasticsearchParseException e) {
if (ignoreMalformed.value() == false) {
throw e;
}
context.addIgnoredField(fieldType.name());
}
parseGeoPointIgnoringMalformed(context, sparse);
token = context.parser().nextToken();
}
} else {
@ -326,35 +319,22 @@ public class GeoPointFieldMapper extends FieldMapper implements ArrayValueMapper
} else {
while (token != XContentParser.Token.END_ARRAY) {
if (token == XContentParser.Token.VALUE_STRING) {
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
parseGeoPointStringIgnoringMalformed(context, sparse);
} else {
try {
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
} catch (ElasticsearchParseException e) {
if (ignoreMalformed.value() == false) {
throw e;
}
}
parseGeoPointIgnoringMalformed(context, sparse);
}
token = context.parser().nextToken();
}
}
}
} else if (token == XContentParser.Token.VALUE_STRING) {
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
parseGeoPointStringIgnoringMalformed(context, sparse);
} else if (token == XContentParser.Token.VALUE_NULL) {
if (fieldType.nullValue() != null) {
parse(context, (GeoPoint) fieldType.nullValue());
}
} else {
try {
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
} catch (ElasticsearchParseException e) {
if (ignoreMalformed.value() == false) {
throw e;
}
context.addIgnoredField(fieldType.name());
}
parseGeoPointIgnoringMalformed(context, sparse);
}
}
@ -362,6 +342,34 @@ public class GeoPointFieldMapper extends FieldMapper implements ArrayValueMapper
return null;
}
/**
* Parses geopoint represented as an object or an array, ignores malformed geopoints if needed
*/
private void parseGeoPointIgnoringMalformed(ParseContext context, GeoPoint sparse) throws IOException {
try {
parse(context, GeoUtils.parseGeoPoint(context.parser(), sparse));
} catch (ElasticsearchParseException e) {
if (ignoreMalformed.value() == false) {
throw e;
}
context.addIgnoredField(fieldType.name());
}
}
/**
* Parses geopoint represented as a string and ignores malformed geopoints if needed
*/
private void parseGeoPointStringIgnoringMalformed(ParseContext context, GeoPoint sparse) throws IOException {
try {
parse(context, sparse.resetFromString(context.parser().text(), ignoreZValue.value()));
} catch (ElasticsearchParseException e) {
if (ignoreMalformed.value() == false) {
throw e;
}
context.addIgnoredField(fieldType.name());
}
}
@Override
protected void doXContentBody(XContentBuilder builder, boolean includeDefaults, Params params) throws IOException {
super.doXContentBody(builder, includeDefaults, params);

13
server/src/test/java/org/elasticsearch/common/geo/GeoHashTests.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.common.geo;
import org.apache.lucene.geo.Rectangle;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.test.ESTestCase;
/**
@ -95,7 +96,17 @@ public class GeoHashTests extends ESTestCase {
Rectangle expectedBbox = GeoHashUtils.bbox(geohash);
Rectangle actualBbox = GeoHashUtils.bbox(extendedGeohash);
assertEquals("Additional data points above 12 should be ignored [" + extendedGeohash + "]" , expectedBbox, actualBbox);
}
}
public void testInvalidGeohashes() {
IllegalArgumentException ex;
ex = expectThrows(IllegalArgumentException.class, () -> GeoHashUtils.mortonEncode("55.5"));
assertEquals("unsupported symbol [.] in geohash [55.5]", ex.getMessage());
ex = expectThrows(IllegalArgumentException.class, () -> GeoHashUtils.mortonEncode(""));
assertEquals("empty geohash", ex.getMessage());
}
}

47
server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java
View File

@ -49,6 +49,7 @@ import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
public class GeoPointFieldMapperTests extends ESSingleNodeTestCase {
@ -398,4 +399,50 @@ public class GeoPointFieldMapperTests extends ESSingleNodeTestCase {
assertThat(defaultValue, not(equalTo(doc.rootDoc().getField("location").binaryValue())));
}
public void testInvalidGeohashIgnored() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties")
.startObject("location")
.field("type", "geo_point")
.field("ignore_malformed", "true")
.endObject()
.endObject().endObject().endObject());
DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser()
.parse("type", new CompressedXContent(mapping));
ParsedDocument doc = defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.field("location", "1234.333")
.endObject()),
XContentType.JSON));
assertThat(doc.rootDoc().getField("location"), nullValue());
}
public void testInvalidGeohashNotIgnored() throws Exception {
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
.startObject("properties")
.startObject("location")
.field("type", "geo_point")
.endObject()
.endObject().endObject().endObject());
DocumentMapper defaultMapper = createIndex("test").mapperService().documentMapperParser()
.parse("type", new CompressedXContent(mapping));
MapperParsingException ex = expectThrows(MapperParsingException.class,
() -> defaultMapper.parse(SourceToParse.source("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
.startObject()
.field("location", "1234.333")
.endObject()),
XContentType.JSON)));
assertThat(ex.getMessage(), equalTo("failed to parse"));
assertThat(ex.getRootCause().getMessage(), equalTo("unsupported symbol [.] in geohash [1234.333]"));
}
}

13
server/src/test/java/org/elasticsearch/index/search/geo/GeoPointParsingTests.java
View File

@ -175,6 +175,19 @@ public class GeoPointParsingTests extends ESTestCase {
assertThat(e.getMessage(), is("field must be either [lat], [lon] or [geohash]"));
}
public void testInvalidGeoHash() throws IOException {
XContentBuilder content = JsonXContent.contentBuilder();
content.startObject();
content.field("geohash", "!!!!");
content.endObject();
XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(content));
parser.nextToken();
Exception e = expectThrows(ElasticsearchParseException.class, () -> GeoUtils.parseGeoPoint(parser));
assertThat(e.getMessage(), is("unsupported symbol [!] in geohash [!!!!]"));
}
private XContentParser objectLatLon(double lat, double lon) throws IOException {
XContentBuilder content = JsonXContent.contentBuilder();
content.startObject();