GNOME Bugzilla – Bug 749963
reverse: Fix nominatim attrs parsing for broken boundingbox
Last modified: 2015-06-09 18:21:47 UTC
.
Created attachment 304070 [details] [review] reverse: Fix nominatim attrs parsing for broken boundingbox This OpenStreeMap result has a bounding box that uses doubles to store its borders, instead of strings, like all the others: { "place_id": "353472", "licence": "Data © OpenStreetMap contributors, ODbL 1.0. http://www.openstreetmap.org/copyright", "osm_type": "node", "osm_id": "151337566", "boundingbox": [ 38.4917175, 38.5317175, -91.1940394, -91.1540394 ], "lat": "38.5117175", "lon": "-91.1740394", "display_name": "Lyon, Franklin County, Missouri, United States of America", "place_rank": "19", "category": "place", "type": "hamlet", We fix this by checking the type of the node, and turning it into a string, as all the other items in the hash table. See https://bugzilla.redhat.com/show_bug.cgi?id=1224563
*** Bug 749961 has been marked as a duplicate of this bug. ***
Review of attachment 304070 [details] [review]: ::: geocode-glib/geocode-reverse.c @@ +125,3 @@ + + bbox_val = json_reader_get_string_value (reader); + g_hash_table_insert(ht, g_strdup (name), g_strdup (bbox_val)); missing space before '(' and below. @@ +133,3 @@ + g_hash_table_insert(ht, g_strdup (name), g_strdup_printf ("%lf", bbox_val)); + json_reader_end_element (reader); + } else { Maybe we should already add handling for other types so that this doesn't happen again at a critical time (like a Fedora release) in future? @@ +135,3 @@ + } else { + g_debug ("Unhandled node type %s for %s", g_type_name (value_type), name); + json_reader_end_element (reader); I don't think ending the element belongs in this function and since we do that in all cases anyway, it should be done by the caller of this func.
Created attachment 304851 [details] [review] reverse: Fix nominatim attrs parsing for broken boundingbox This OpenStreeMap result has a bounding box that uses doubles to store its borders, instead of strings, like all the others: { "place_id": "353472", "licence": "Data © OpenStreetMap contributors, ODbL 1.0. http://www.openstreetmap.org/copyright", "osm_type": "node", "osm_id": "151337566", "boundingbox": [ 38.4917175, 38.5317175, -91.1940394, -91.1540394 ], "lat": "38.5117175", "lon": "-91.1740394", "display_name": "Lyon, Franklin County, Missouri, United States of America", "place_rank": "19", "category": "place", "type": "hamlet", We fix this by checking the type of the node, and turning it into a string, as all the other items in the hash table. See https://bugzilla.redhat.com/show_bug.cgi?id=1224563
Review of attachment 304070 [details] [review]: ::: geocode-glib/geocode-reverse.c @@ +125,3 @@ + + bbox_val = json_reader_get_string_value (reader); + g_hash_table_insert(ht, g_strdup (name), g_strdup (bbox_val)); Fixed. @@ +133,3 @@ + g_hash_table_insert(ht, g_strdup (name), g_strdup_printf ("%lf", bbox_val)); + json_reader_end_element (reader); + } else { I've added support for G_TYPE_INT64, but we can't parse NULL, or BOOLEANs. We dump a debug output. We could output a warning, but I don't think we should warn when parsing data we don't have a direct control over. @@ +135,3 @@ + } else { + g_debug ("Unhandled node type %s for %s", g_type_name (value_type), name); + json_reader_end_element (reader); That's going to get ugly, but sure.
Is this releavant to this bug? https://trac.openstreetmap.org/ticket/5132
(In reply to Jonas Danielsson from comment #6) > Is this releavant to this bug? > > https://trac.openstreetmap.org/ticket/5132 Completely. Want me to reference it in the commit log?
Yeah, I think that would be nice. Thanks.
With the upstream OSM bug referenced. Thanks. Attachment 304851 [details] pushed as e11087f - reverse: Fix nominatim attrs parsing for broken boundingbox
*** Bug 750588 has been marked as a duplicate of this bug. ***