Fix a bug with nodes outside bbox

If there are nodes with locations outside the bounding box we are interested in
in the input osm file and we are generating the geodistribution for ways images,
tagstats could segfault. In this case the node location wasn't stored in the
m_storage, so when reading the it back, either you get an unintialized value
or a segfault. (This also depends on the type of storage used.)
Thanks to @lonvia for help debugging this.

The solution is to store "MAXINT" as a marker and make sure it doesn't end up
in the GeoDistribution.

2 files changed, 12 insertions, 12 deletions

diff --git a/tagstats/geodistribution.hpp b/tagstats/geodistribution.hpp
index 8c72035..d5d1c78 100644
--- a/tagstats/geodistribution.hpp
+++ b/tagstats/geodistribution.hpp
@@ -32,6 +32,9 @@
  * Osmium::OSM::Position to a bounding box, reduces the resolution
  * of the coordinates and returns an integer.
  *
+ * If the position is outside the bounding box, std::numeric_limits<T>::max()
+ * is returned.
+ *
  * @tparam T Result type after conversion. Must be an unsigned integer type.
  */
 template <typename T>
@@ -54,14 +57,15 @@ public:
         m_minx(minx), m_miny(miny), m_maxx(maxx), m_maxy(maxy),
         m_width(width), m_height(height),
         m_dx(maxx - minx), m_dy(maxy - miny) {
-        if (size() > std::numeric_limits<T>::max()) {
+        if (size() >= std::numeric_limits<T>::max()) {
             throw std::range_error("width*height must be smaller than MAXINT for type T");
         }
     }
 
     T operator()(const Osmium::OSM::Position& p) const {
         if (p.lon() < m_minx || p.lat() < m_miny || p.lon() >= m_maxx || p.lat() >= m_maxy) {
-            throw std::range_error("position out of bounds");
+            // if the position is out of bounds we return MAXINT for type T
+            return std::numeric_limits<T>::max();
         }
         int x = (p.lon() - m_minx) / m_dx * m_width;
         int y = (m_maxy - p.lat()) / m_dy * m_height;
@@ -149,6 +153,10 @@ public:
      * Add the given coordinate to the distribution store.
      */
     void add_coordinate(rough_position_t n) {
+        if (n == std::numeric_limits<rough_position_t>::max()) {
+            // ignore positions that are out of bounds
+            return;
+        }
         if (m_cells == 0) {
             m_location = n;
             m_cells++;
diff --git a/tagstats/tagstats_handler.hpp b/tagstats/tagstats_handler.hpp
index 4049083..d3ea136 100644
--- a/tagstats/tagstats_handler.hpp
+++ b/tagstats/tagstats_handler.hpp
@@ -412,11 +412,7 @@ class TagStatsHandler : public Osmium::Handler::Base {
             stat->update(it->value(), object, m_string_store);
 
             if (object.type() == NODE) {
-                try {
-                    stat->distribution.add_coordinate(m_map_to_int(static_cast<const Osmium::OSM::Node&>(object).position()));
-                } catch (std::range_error) {
-                    // ignore
-                }
+                stat->distribution.add_coordinate(m_map_to_int(static_cast<const Osmium::OSM::Node&>(object).position()));
             }
 #ifdef TAGSTATS_GEODISTRIBUTION_FOR_WAYS
             else if (object.type() == WAY) {
@@ -479,11 +475,7 @@ public:
         statistics_handler.node(node);
         collect_tag_stats(*node);
 #ifdef TAGSTATS_GEODISTRIBUTION_FOR_WAYS
-        try {
-            m_storage.set(node->id(), m_map_to_int(node->position()));
-        } catch (std::range_error) {
-            // ignore
-        }
+        m_storage.set(node->id(), m_map_to_int(node->position()));
 #endif
     }