Prevent crashing when computing neighbors

* Raise error on invalid geocode characters. * Prevent recursing past end of string. fixes transitland#4
Vince-Smith · May 8, 2019 · 6f483ce · 6f483ce
1 parent 11bd8e1
commit 6f483ce
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 6 deletions.
diff --git a/ext/geohash_native.c b/ext/geohash_native.c
@@ -27,8 +27,10 @@
 
 #include "ruby.h"
 #include <ctype.h>
+#include <string.h>
 
 static VALUE rb_cGeoHash;
+static VALUE rb_eInvalidGeoHashError;
 
 #define BASE32	"0123456789bcdefghjkmnpqrstuvwxyz"
 
@@ -61,6 +63,10 @@ static void decode_geohash_bbox(char *geohash, double *lat, double *lon) {
 }
 
 static void decode_geohash(char *geohash, double *point) {
+	if (strspn(geohash, BASE32) != strlen(geohash)) {
+		rb_raise(rb_eInvalidGeoHashError, "Invalid character.");
+	}
+
 	double lat[2], lon[2];
 
 	decode_geohash_bbox(geohash, lat, lon);
@@ -170,18 +176,18 @@ void get_neighbor(char *str, int dir, int hashlen)
 	/* Right, Left, Top, Bottom */
 
 	static char *neighbors[] = { "bc01fg45238967deuvhjyznpkmstqrwx",
-														 	 "238967debc01fg45kmstqrwxuvhjyznp",
-														 	 "p0r21436x8zb9dcf5h7kjnmqesgutwvy",
-														   "14365h7k9dcfesgujnmqp0r2twvyx8zb" };
-												
+	                             "238967debc01fg45kmstqrwxuvhjyznp",
+	                             "p0r21436x8zb9dcf5h7kjnmqesgutwvy",
+	                             "14365h7k9dcfesgujnmqp0r2twvyx8zb" };
+
 	static char *borders[] = { "bcfguvyz", "0145hjnp", "prxz", "028b" };
 
 	char last_chr, *border, *neighbor;
 	int index = ( 2 * (hashlen % 2) + dir) % 4;
 	neighbor = neighbors[index];
 	border = borders[index];
 	last_chr = str[hashlen-1];
-	if (strchr(border,last_chr))
+	if (strchr(border,last_chr) && hashlen != 0)
 		get_neighbor(str, dir, hashlen-1);
 	str[hashlen-1] = BASE32[strchr(neighbor, last_chr)-neighbor];
 }
@@ -203,7 +209,11 @@ static VALUE calculate_adjacent(VALUE self, VALUE geohash, VALUE dir)
 void Init_geohash_native()
 {
 	rb_cGeoHash = rb_define_class("GeoHash", rb_cObject);
+	rb_eInvalidGeoHashError = rb_define_class_under(rb_cGeoHash, "InvalidGeoHashError", rb_eStandardError);
+
+	rb_define_attr(rb_eInvalidGeoHashError, "additional_info", 1, 0);
 	rb_define_singleton_method(rb_cGeoHash, "decode_bbox", decode_bbox, 1);
+
 	rb_define_singleton_method(rb_cGeoHash, "decode_base", decode, 1);
 	rb_define_singleton_method(rb_cGeoHash, "encode_base", encode, 3);
 	rb_define_singleton_method(rb_cGeoHash, "calculate_adjacent", calculate_adjacent, 2);

diff --git a/lib/geohash.rb b/lib/geohash.rb
@@ -24,7 +24,7 @@ def self.encode(lat, lon, precision=10)
 
   # Decode a geohash to a latitude and longitude with decimals digits
   def self.decode(geohash, decimals=5)
-    lat, lon = decode_base(geohash)
+    lat, lon = decode_base(geohash.downcase)
     [lat.decimals(decimals), lon.decimals(decimals)]
   end
 

diff --git a/test/test_geohash.rb b/test/test_geohash.rb
@@ -15,6 +15,12 @@ def test_decoding
     assert_equal [37.8565, -122.2554], GeoHash.decode("9q9p658642g7", 4)
   end
 
+  def test_invalid_decoding
+    assert_raises GeoHash::InvalidGeoHashError do
+     GeoHash.decode("a")
+    end
+  end
+
   def test_encoding
     assert_equal "dqcw4bnrs6s7", GeoHash.encode(39.0247389581054, -76.5110040642321, 12)
     assert_equal "dqcw4bnrs6", GeoHash.encode(39.0247389581054, -76.5110040642321, 10)
@@ -52,6 +58,7 @@ def test_specific_bbox
 
   def test_neighbors
     assert_equal ["dqcjr1", "dqcjq9", "dqcjqf", "dqcjqb", "dqcjr4", "dqcjr0", "dqcjqd", "dqcjq8"], GeoHash.new("dqcjqc").neighbors
+    assert_equal ["9", "x", "b", "2", "c", "3", "z", "r"].sort, GeoHash.new("8").neighbors.sort
 
     assert_equal "dqcw5", GeoHash.calculate_adjacent("dqcw4", 0)  # right
     assert_equal "dqcw1", GeoHash.calculate_adjacent("dqcw4", 1)  # left
@@ -65,6 +72,17 @@ def test_neighbors
     assert_equal 8, (["dqcw7", "dqctg", "dqcw4", "dqcwh", "dqcw6", "dqcwk", "dqctf", "dqctu"] & GeoHash.new("dqcw5").neighbors).size
   end
 
+  def test_edge_neighbors
+    assert_equal ["852", "853", "851", "84c", "84b", "xfz", "xgp", "xgr"].sort, GeoHash.new("850").neighbors.sort
+
+    assert_equal "851", GeoHash.calculate_adjacent("850", 0)  # right
+    assert_equal "xgp", GeoHash.calculate_adjacent("850", 1)  # left
+    assert_equal "852", GeoHash.calculate_adjacent("850", 2)  # top
+    assert_equal "84b", GeoHash.calculate_adjacent("850", 3)  # bottom
+
+    assert_equal 8, (["852", "853", "851", "84c", "84b", "xfz", "xgp", "xgr"] & GeoHash.new("850").neighbors).size
+  end
+
   # require 'benchmark'
   # def test_multiple
   #   Benchmark.bmbm(30) do |bm|