Skip to content

Commit

Permalink
Fix overflow error in parsing of long geohashes (#29418)
Browse files Browse the repository at this point in the history
Fixes a possible overflow error that geohashes longer than 12 characters
can cause during parsing.

Fixes #24616
  • Loading branch information
imotov committed Apr 16, 2018
1 parent 9cfc6ca commit c12a424
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
10 changes: 10 additions & 0 deletions docs/reference/mapping/types/geo-point.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ format was changed early on to conform to the format used by GeoJSON.
==================================================

[NOTE]
A point can be expressed as a http://en.wikipedia.org/wiki/Geohash[geohash].
Geohashes are https://en.wikipedia.org/wiki/Base32[base32] encoded strings of
the bits of the latitude and longitude interleaved. Each character in a geohash
adds additional 5 bits to the precision. So the longer the hash, the more
precise it is. For the indexing purposed geohashs are translated into
latitude-longitude pairs. During this process only first 12 characters are
used, so specifying more than 12 characters in a geohash doesn't increase the
precision. The 12 characters provide 60 bits, which should reduce a possible
error to less than 2cm.

[[geo-point-params]]
==== Parameters for `geo_point` fields
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,19 @@ public static final long longEncode(final double lon, final double lat, final in
/**
* Encode from geohash string to the geohash based long format (lon/lat interleaved, 4 least significant bits = level)
*/
public static final long longEncode(final String hash) {
int level = hash.length()-1;
private static long longEncode(final String hash, int length) {
int level = length - 1;
long b;
long l = 0L;
for(char c : hash.toCharArray()) {
b = (long)(BASE_32_STRING.indexOf(c));
l |= (b<<(level--*5));
if (level < 0) {
// We cannot handle more than 12 levels
break;
}
}
return (l<<4)|hash.length();
return (l << 4) | length;
}

/**
Expand Down Expand Up @@ -173,6 +177,10 @@ public static final long mortonEncode(final String hash) {
for(char c : hash.toCharArray()) {
b = (long)(BASE_32_STRING.indexOf(c));
l |= (b<<((level--*5) + MORTON_OFFSET));
if (level < 0) {
// We cannot handle more than 12 levels
break;
}
}
return BitUtil.flipFlop(l);
}
Expand Down Expand Up @@ -200,13 +208,14 @@ private static char encode(int x, int y) {
public static Rectangle bbox(final String geohash) {
// bottom left is the coordinate
GeoPoint bottomLeft = GeoPoint.fromGeohash(geohash);
long ghLong = longEncode(geohash);
int len = Math.min(12, geohash.length());
long ghLong = longEncode(geohash, len);
// shift away the level
ghLong >>>= 4;
// deinterleave and add 1 to lat and lon to get topRight
long lat = BitUtil.deinterleave(ghLong >>> 1) + 1;
long lon = BitUtil.deinterleave(ghLong) + 1;
GeoPoint topRight = GeoPoint.fromGeohash(BitUtil.interleave((int)lon, (int)lat) << 4 | geohash.length());
GeoPoint topRight = GeoPoint.fromGeohash(BitUtil.interleave((int)lon, (int)lat) << 4 | len);

return new Rectangle(bottomLeft.lat(), topRight.lat(), bottomLeft.lon(), topRight.lon());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,20 @@ public void testGeohashExtremes() {
assertEquals("xbpbpbpbpbpb", GeoHashUtils.stringEncode(180, 0));
assertEquals("zzzzzzzzzzzz", GeoHashUtils.stringEncode(180, 90));
}

public void testLongGeohashes() {
for (int i = 0; i < 100000; i++) {
String geohash = randomGeohash(12, 12);
GeoPoint expected = GeoPoint.fromGeohash(geohash);
// Adding some random geohash characters at the end
String extendedGeohash = geohash + randomGeohash(1, 10);
GeoPoint actual = GeoPoint.fromGeohash(extendedGeohash);
assertEquals("Additional data points above 12 should be ignored [" + extendedGeohash + "]" , expected, actual);

Rectangle expectedBbox = GeoHashUtils.bbox(geohash);
Rectangle actualBbox = GeoHashUtils.bbox(extendedGeohash);
assertEquals("Additional data points above 12 should be ignored [" + extendedGeohash + "]" , expectedBbox, actualBbox);

}
}
}

0 comments on commit c12a424

Please sign in to comment.