Update changelog and bump to 5.2.7

CHANGELOG.md

@@ -1,3 +1,16 @@
+# 5.2.7
+  - Bugfixes
+    - BREAKING: Fix bug that could result in failure to load 'osrm.icd' files. This breaks the dataformat
+    - Fix deduplication of parallel edges. Now uses correct weight.
+
+# 5.2.6
+  - Bugfixes
+    - Fix numeric overflow in roundabout center calculation which throws an exception
+
+# 5.2.5
+  - Bugfixes
+    - Fixes a segfault caused by incorrect trimming logic for very short steps.
+
 # 5.2.4
   Changes from 5.2.3:
   - Bugfixes:

CMakeLists.txt

@@ -10,7 +10,7 @@ endif()
 project(OSRM C CXX)
 set(OSRM_VERSION_MAJOR 5)
 set(OSRM_VERSION_MINOR 2)
-set(OSRM_VERSION_PATCH 0)
+set(OSRM_VERSION_PATCH 7)
 
 # these two functions build up custom variables:
 #   OSRM_INCLUDE_PATHS and OSRM_DEFINES

features/step_definitions/data.js

@@ -17,7 +17,7 @@ module.exports = function () {
         this.setContractArgs(args, callback);
     });
 
-    this.Given(/^a grid size of (\d+) meters$/, (meters, callback) => {
+    this.Given(/^a grid size of ([0-9.]+) meters$/, (meters, callback) => {
         this.setGridSize(meters);
         callback();
     });

features/testbot/basic.feature

@@ -233,7 +233,7 @@ Feature: Basic Routing
             | d    | a  | abcd,abcd  |
             | a    | m  | aeim,aeim  |
             | m    | a  | aeim,aeim  |
-    
+
     Scenario: Testbot - Triangle challenge
         Given the node map
             |   |   |   | d |
@@ -251,3 +251,18 @@ Feature: Basic Routing
             | from | to | route |
             | d    | c  | de,ce,ce |
             | e    | d  | de,de    |
+
+    Scenario: Ambiguous edge weights - Use minimal edge weight
+        Given the node map
+            | a | b |
+
+        And the ways
+            | nodes | highway   | name |
+            | ab    | tertiary  |      |
+            | ab    | primary   |      |
+            | ab    | secondary |      |
+
+        When I route I should get
+            | from | to | route | time |
+            | a    | b  | ,     | 10s  |
+            | b    | a  | ,     | 10s  |

features/testbot/fixed.feature

@@ -24,3 +24,18 @@ Feature: Fixed bugs, kept to check for regressions
         When I route I should get
             | from | to | route   |
             | x    | y  | abc,abc |
+
+    Scenario: Step trimming with very short segments
+        Given a grid size of 0.1 meters
+        Given the node map
+            | a | 1 | b | c | d | 2 | e |
+
+        Given the ways
+            | nodes | oneway |
+            | ab    | yes    |
+            | bcd   | yes    |
+            | de    | yes    |
+
+        When I route I should get
+            | from | to | route     |
+            | 1    | 2  | bcd,bcd   |

include/engine/datafacade/internal_datafacade.hpp

@@ -330,7 +330,7 @@ class InternalDataFacade final : public BaseDataFacade
             std::vector<util::guidance::BearingClass> bearing_classes;
             // and the actual bearing values
             std::uint64_t num_bearings;
-            intersection_stream >> num_bearings;
+            intersection_stream.read(reinterpret_cast<char*>(&num_bearings),sizeof(num_bearings));
             m_bearing_values_table.resize(num_bearings);
             intersection_stream.read(reinterpret_cast<char *>(&m_bearing_values_table[0]),
                                      sizeof(m_bearing_values_table[0]) * num_bearings);

include/util/guidance/entry_class.hpp

@@ -62,6 +62,13 @@ class EntryClass
     friend std::size_t std::hash<EntryClass>::operator()(const EntryClass &) const;
 };
 
+#if not defined __GNUC__ or __GNUC__ > 4
+static_assert(std::is_trivially_copyable<EntryClass>::value,
+              "Class is serialized trivially in "
+              "the datafacades. Bytewise writing "
+              "requires trivially copyable type");
+#endif
+
 } // namespace guidance
 } // namespace utilr
 } // namespace osrm

src/engine/guidance/post_processing.cpp

@@ -928,13 +928,13 @@ void trimShortSegments(std::vector<RouteStep> &steps, LegGeometry &geometry)
     auto &next_to_last_step = *(steps.end() - 2);
     // in the end, the situation with the roundabout cannot occur. As a result, we can remove
     // all zero-length instructions
-    if (next_to_last_step.distance <= 1)
+    if (next_to_last_step.distance <= 1 && steps.size() > 2)
     {
         geometry.locations.pop_back();
         geometry.annotations.pop_back();
         geometry.osm_node_ids.pop_back();
         geometry.segment_offsets.pop_back();
-        BOOST_ASSERT(geometry.segment_distances.back() < 1);
+        BOOST_ASSERT(geometry.segment_distances.back() <= 1);
         geometry.segment_distances.pop_back();
 
         next_to_last_step.maneuver.waypoint_type = WaypointType::Arrive;

src/extractor/extraction_containers.cpp

@@ -417,11 +417,13 @@ void ExtractionContainers::PrepareEdges(lua_State *segment_state)
                 all_edges_list[i].result.weight < min_forward_weight)
             {
                 min_forward_idx = i;
+                min_forward_weight = all_edges_list[i].result.weight;
             }
             if (all_edges_list[i].result.backward &&
                 all_edges_list[i].result.weight < min_backward_weight)
             {
                 min_backward_idx = i;
+                min_backward_weight = all_edges_list[i].result.weight;
             }
 
             // this also increments the outer loop counter!

src/extractor/extractor.cpp

@@ -661,7 +661,7 @@ void Extractor::WriteIntersectionClassificationData(
     util::RangeTable<> bearing_class_range_table(bearing_counts);
     file_out_stream << bearing_class_range_table;
 
-    file_out_stream << total_bearings;
+    file_out_stream.write(reinterpret_cast<const char *>(&total_bearings), sizeof(total_bearings));
     for (const auto &bearing_class : bearing_classes)
     {
         const auto &bearings = bearing_class.getAvailableBearings();
@@ -669,17 +669,18 @@ void Extractor::WriteIntersectionClassificationData(
                               sizeof(bearings[0]) * bearings.size());
     }
 
-    // FIXME
-    // This should be here, but g++4.8 does not have it...
-    // static_assert(std::is_trivially_copyable<util::guidance::EntryClass>::value,
-    //              "EntryClass Serialization requires trivial copyable entry classes");
+    if (!static_cast<bool>(file_out_stream))
+    {
+        throw util::exception("Failed to write to " + output_file_name + ".");
+    }
 
     util::serializeVector(file_out_stream, entry_classes);
     TIMER_STOP(write_edges);
     util::SimpleLogger().Write() << "ok, after " << TIMER_SEC(write_edges) << "s for "
                                  << node_based_intersection_classes.size() << " Indices into "
                                  << bearing_classes.size() << " bearing classes and "
-                                 << entry_classes.size() << " entry classes";
+                                 << entry_classes.size() << " entry classes and " << total_bearings
+                                 << " bearing values." << std::endl;
 }
 }
 }

src/storage/storage.cpp

@@ -360,7 +360,7 @@ int Storage::Run()
     }
 
     std::uint64_t num_bearings;
-    intersection_stream >> num_bearings;
+    intersection_stream.read(reinterpret_cast<char*>(&num_bearings),sizeof(num_bearings));
 
     std::vector<DiscreteBearing> bearing_class_table(num_bearings);
     intersection_stream.read(reinterpret_cast<char *>(&bearing_class_table[0]),

src/util/coordinate_calculation.cpp

@@ -258,7 +258,10 @@ circleCenter(const Coordinate C1, const Coordinate C2, const Coordinate C3)
                             C2C1_slope * (C2_x + C3_x)) /
                            (2 * (C3C2_slope - C2C1_slope));
         const double lat = (0.5 * (C1_x + C2_x) - lon) / C2C1_slope + 0.5 * (C1_y + C2_y);
-        return Coordinate(FloatLongitude(lon), FloatLatitude(lat));
+        if (lon < -180.0 || lon > 180.0 || lat < -90.0 || lat > 90.0)
+            return boost::none;
+        else
+            return Coordinate(FloatLongitude(lon), FloatLatitude(lat));
     }
 }
 

unit_tests/util/coordinate_calculation.cpp

@@ -302,6 +302,13 @@ BOOST_AUTO_TEST_CASE(circleCenter)
     c = Coordinate(FloatLongitude(-112.096419), FloatLatitude(41.147259));
     result = coordinate_calculation::circleCenter(a, b, c);
     BOOST_CHECK(!result);
+
+    // Out of bounds
+    a = Coordinate(FloatLongitude(-112.096234), FloatLatitude(41.147258));
+    b = Coordinate(FloatLongitude(-112.106606), FloatLatitude(41.147259));
+    c = Coordinate(FloatLongitude(-113.096419), FloatLatitude(41.147258));
+    result = coordinate_calculation::circleCenter(a, b, c);
+    BOOST_CHECK(!result);
 }
 
 BOOST_AUTO_TEST_SUITE_END()