Fix crash in MLD alternative search if source or target are invalid

In situations where there is not a valid source or target phantom node (e.g. when snapping to an edge with a zero weight), a heap assertion will fail in the MLD alternative search code. We fix this by checking for empty heaps before proceeding with the search.
Project-OSRM · Oct 8, 2020 · ffa0709 · ffa0709
1 parent e033e0b
commit ffa0709
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@
       - FIXED: treat `bicycle=use_sidepath` as no access on the tagged way. [#5622](https://github.com/Project-OSRM/osrm-backend/pull/5622)
       - FIXED: fix table result when source and destination on same one-way segment. [#5828](https://github.com/Project-OSRM/osrm-backend/pull/5828)
       - FIXED: fix occasional segfault when swapping data with osrm-datastore and using `exclude=` [#5844](https://github.com/Project-OSRM/osrm-backend/pull/5844)
+      - FIXED: fix crash in MLD alternative search if source or target are invalid [#5851](https://github.com/Project-OSRM/osrm-backend/pull/5851)
     - Misc:
       - CHANGED: Reduce memory usage for raster source handling. [#5572](https://github.com/Project-OSRM/osrm-backend/pull/5572)
       - CHANGED: Add cmake option `ENABLE_DEBUG_LOGGING` to control whether output debug logging. [#3427](https://github.com/Project-OSRM/osrm-backend/issues/3427)

diff --git a/features/testbot/zero-speed-updates.feature b/features/testbot/zero-speed-updates.feature
@@ -93,6 +93,31 @@ Feature: Check zero speed updates
           |    1 |  2 | NoRoute |
 
 
+    Scenario: Routing with alternatives on restricted way
+        Given the node map
+            """
+            a-1-b-2-c
+            """
+
+        And the ways
+            | nodes | oneway |
+            | abc   | no     |
+        And the contract extra arguments "--segment-speed-file {speeds_file}"
+        And the customize extra arguments "--segment-speed-file {speeds_file}"
+        And the speed file
+            """
+            1,2,0
+            2,1,0
+            """
+        And the query options
+            | alternatives | true |
+
+
+        When I route I should get
+          | from | to | code    | alternative |
+          |    1 |  2 | NoRoute |             |
+
+
     Scenario: Routing on restricted oneway
         Given the node map
             """

diff --git a/src/engine/routing_algorithms/alternative_path_mld.cpp b/src/engine/routing_algorithms/alternative_path_mld.cpp
@@ -662,7 +662,15 @@ makeCandidateVias(SearchEngineData<Algorithm> &search_engine_data,
     Heap &forward_heap = *search_engine_data.forward_heap_1;
     Heap &reverse_heap = *search_engine_data.reverse_heap_1;
 
+    // All via nodes in the overlapping search space (except the shortest path via node).
+    // Will be filtered and ranked and then used for s,via and via,t alternative paths.
+    std::vector<WeightedViaNode> candidate_vias;
+
     insertNodesInHeaps(forward_heap, reverse_heap, phantom_node_pair);
+    if (forward_heap.Empty() || reverse_heap.Empty())
+    {
+        return candidate_vias;
+    }
 
     // The single via node in the shortest paths s,via and via,t sub-paths and
     // the weight for the shortest path s,t we return and compare alternatives to.
@@ -673,10 +681,6 @@ makeCandidateVias(SearchEngineData<Algorithm> &search_engine_data,
     NodeID overlap_via = SPECIAL_NODEID;
     EdgeWeight overlap_weight = INVALID_EDGE_WEIGHT;
 
-    // All via nodes in the overlapping search space (except the shortest path via node).
-    // Will be filtered and ranked and then used for s,via and via,t alternative paths.
-    std::vector<WeightedViaNode> candidate_vias;
-
     // The logic below is a bit weird - here's why: we want to re-use the MLD routingStep for
     // stepping our search space from s and from t. We don't know how far to overlap until we have
     // the shortest path. Once we have the shortest path we can use its weight to terminate when