Open-Harbor/osrm-backend
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

Fix fallback speed validity checks (#5300)

Browse Source 
* fix fallback_speeds check to only accept values > 0

* add invalid_fallback_speed
This commit is contained in:
Kajari Ghosh 2018-12-10 14:53:30 -05:00 committed by GitHub
parent 2e17f3010a
commit 01ca32c81c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 157 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
features/testbot/distance_matrix.feature
View File

@ -596,7 +596,6 @@ Feature: Basic Distance Matrix
| e | 198.8 | 298.9 | 499 | 710.3 | 0 | 1592.8 | | e | 198.8 | 298.9 | 499 | 710.3 | 0 | 1592.8 |
| f | 596.4 | 696.5 | 896.6 | 1107.9 | 397.6 | 0 | | f | 596.4 | 696.5 | 896.6 | 1107.9 | 397.6 | 0 |
Scenario: Testbot - Filling in noroutes with estimates (defaults to input coordinate location) Scenario: Testbot - Filling in noroutes with estimates (defaults to input coordinate location)
Given a grid size of 300 meters Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4" Given the extract extra arguments "--small-component-size 4"
@ -620,7 +619,18 @@ Feature: Basic Distance Matrix
| f | 900.7 | 600.5 | 0 | 302.2 | | f | 900.7 | 600.5 | 0 | 302.2 |
| 1 | 1501.1 | 1200.9 | 300.2 | 0 | | 1 | 1501.1 | 1200.9 | 300.2 | 0 |
Scenario: Testbot - Filling in noroutes with estimates - use input coordinate When I request a travel distance matrix I should get
| | a | b | f | 1 |
| a | 0 | 300.2 | 900.7 | 1501.1 |
When I request a travel distance matrix I should get
| | a |
| a | 0 |
| b | 300.2 |
| f | 900.7 |
| 1 | 1501.1 |
Scenario: Testbot - Fise input coordinate
Given a grid size of 300 meters Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4" Given the extract extra arguments "--small-component-size 4"
Given the query options Given the query options
@ -644,6 +654,18 @@ Feature: Basic Distance Matrix
| f | 900.7 | 600.5 | 0 | 302.2 | | f | 900.7 | 600.5 | 0 | 302.2 |
| 1 | 1501.1 | 1200.9 | 300.2 | 0 | | 1 | 1501.1 | 1200.9 | 300.2 | 0 |
When I request a travel distance matrix I should get
| | a | b | f | 1 |
| a | 0 | 300.2 | 900.7 | 1501.1 |
When I request a travel distance matrix I should get
| | a |
| a | 0 |
| b | 300.2 |
| f | 900.7 |
| 1 | 1501.1 |
Scenario: Testbot - Filling in noroutes with estimates - use snapped coordinate Scenario: Testbot - Filling in noroutes with estimates - use snapped coordinate
Given a grid size of 300 meters Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4" Given the extract extra arguments "--small-component-size 4"
@ -668,22 +690,9 @@ Feature: Basic Distance Matrix
| f | 900.7 | 600.5 | 0 | 302.2 | | f | 900.7 | 600.5 | 0 | 302.2 |
| 1 | 1200.9 | 900.7 | 300.2 | 0 | | 1 | 1200.9 | 900.7 | 300.2 | 0 |
Scenario: Testbot - Asymetric fallback_speed - more sources than destinations When I request a travel distance matrix I should get
Given a grid size of 300 meters | | a | b | f | 1 |
Given the extract extra arguments "--small-component-size 4" | a | 0 | 300.2 | 900.7 | 1200.9 |
Given the query options
| fallback_speed | 5 |
| fallback_coordinate | snapped |
Given the node map
"""
a b f h 1
d e g i
"""
And the ways
| nodes |
| abeda |
| fhigf |
When I request a travel distance matrix I should get When I request a travel distance matrix I should get
| | a | | | a |
@ -692,23 +701,3 @@ Feature: Basic Distance Matrix
| f | 900.7 | | f | 900.7 |
| 1 | 1200.9 | | 1 | 1200.9 |
Scenario: Testbot - Asymetric fallback_speed - more destinations than sources
Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4"
Given the query options
| fallback_speed | 5 |
| fallback_coordinate | snapped |
Given the node map
"""
a b f h 1
d e g i
"""
And the ways
| nodes |
| abeda |
| fhigf |
When I request a travel distance matrix I should get
| | a | b | f | 1 |
| a | 0 | 300.2 | 900.7 | 1200.9 |

73
features/testbot/duration_matrix.feature
View File

@ -534,6 +534,17 @@ Feature: Basic Duration Matrix
| f | 18 | 12 | 0 | 30 | | f | 18 | 12 | 0 | 30 |
| 1 | 30 | 24 | 30 | 0 | | 1 | 30 | 24 | 30 | 0 |
When I request a travel time matrix I should get
| | a | b | f | 1 |
| a | 0 | 30 | 18 | 30 |
When I request a travel time matrix I should get
| | a |
| a | 0 |
| b | 30 |
| f | 18 |
| 1 | 30 |
Scenario: Testbot - Filling in noroutes with estimates - use input coordinate Scenario: Testbot - Filling in noroutes with estimates - use input coordinate
Given a grid size of 300 meters Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4" Given the extract extra arguments "--small-component-size 4"
@ -558,6 +569,17 @@ Feature: Basic Duration Matrix
| f | 18 | 12 | 0 | 30 | | f | 18 | 12 | 0 | 30 |
| 1 | 30 | 24 | 30 | 0 | | 1 | 30 | 24 | 30 | 0 |
When I request a travel time matrix I should get
| | a | b | f | 1 |
| a | 0 | 30 | 18 | 30 |
When I request a travel time matrix I should get
| | a |
| a | 0 |
| b | 30 |
| f | 18 |
| 1 | 30 |
Scenario: Testbot - Filling in noroutes with estimates - use snapped coordinate Scenario: Testbot - Filling in noroutes with estimates - use snapped coordinate
Given a grid size of 300 meters Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4" Given the extract extra arguments "--small-component-size 4"
@ -582,25 +604,33 @@ Feature: Basic Duration Matrix
| f | 18 | 12 | 0 | 30 | | f | 18 | 12 | 0 | 30 |
| 1 | 24 | 18 | 30 | 0 | | 1 | 24 | 18 | 30 | 0 |
When I request a travel time matrix I should get
| | a | b | f | 1 |
| a | 0 | 30 | 18 | 24 |
When I request a travel time matrix I should get
| | a |
| a | 0 |
| b | 30 |
| f | 18 |
| 1 | 24 |
Scenario: Testbot - Travel time matrix of minimal network with scale factor Scenario: Testbot - Travel time matrix of minimal network with scale factor
Given the query options Given the query options
| scale_factor | 2 | | scale_factor | 2 |
Given the node map
Given the node map
""" """
a b a b
""" """
And the ways
And the ways
| nodes | | nodes |
| ab | | ab |
When I request a travel time matrix I should get
When I request a travel time matrix I should get
| | a | b | | | a | b |
| a | 0 | 20 | | a | 0 | 20 |
| b | 20 | 0 | | b | 20 | 0 |
Scenario: Testbot - Test fallback speeds and scale factor
Scenario: Testbot - Test fallback speeds and scale factor
Given a grid size of 300 meters Given a grid size of 300 meters
Given the extract extra arguments "--small-component-size 4" Given the extract extra arguments "--small-component-size 4"
Given the query options Given the query options
@ -608,68 +638,67 @@ Feature: Basic Duration Matrix
| fallback_speed | 5 | | fallback_speed | 5 |
| fallback_coordinate | snapped | | fallback_coordinate | snapped |
Given the node map Given the node map
""" """
a b f h 1 a b f h 1
d e g i d e g i
""" """
And the ways And the ways
| nodes | | nodes |
| abeda | | abeda |
| fhigf | | fhigf |
When I request a travel time matrix I should get When I request a travel time matrix I should get
| | a | b | f | 1 | | | a | b | f | 1 |
| a | 0 | 60 | 36 | 48 | | a | 0 | 60 | 36 | 48 |
| b | 60 | 0 | 24 | 36 | | b | 60 | 0 | 24 | 36 |
| f | 36 | 24 | 0 | 60 | | f | 36 | 24 | 0 | 60 |
| 1 | 48 | 36 | 60 | 0 | | 1 | 48 | 36 | 60 | 0 |
When I request a travel time matrix I should get When I request a travel time matrix I should get
| | a | b | f | 1 | | | a | b | f | 1 |
| a | 0 | 60 | 36 | 48 | | a | 0 | 60 | 36 | 48 |
When I request a travel time matrix I should get When I request a travel time matrix I should get
| | a | | | a |
| a | 0 | | a | 0 |
| b | 60 | | b | 60 |
| f | 36 | | f | 36 |
| 1 | 48 | | 1 | 48 |
Scenario: Testbot - Travel time matrix of minimal network with overflow scale factor
Scenario: Testbot - Travel time matrix of minimal network with overflow scale factor
Given the query options Given the query options
| scale_factor | 2147483647 | | scale_factor | 2147483647 |
Given the node map Given the node map
""" """
a b a b
""" """
And the ways And the ways
| nodes | | nodes |
| ab | | ab |
When I request a travel time matrix I should get When I request a travel time matrix I should get
| | a | b | | | a | b |
| a | 0 | 214748364.6 | | a | 0 | 214748364.6 |
| b | 214748364.6 | 0 | | b | 214748364.6 | 0 |
Scenario: Testbot - Travel time matrix of minimal network with fraction scale factor Scenario: Testbot - Travel time matrix of minimal network with fraction scale factor
Given the query options Given the query options
| scale_factor | 0.5 | | scale_factor | 0.5 |
Given the node map Given the node map
""" """
a b a b
""" """
And the ways And the ways
| nodes | | nodes |
| ab | | ab |
When I request a travel time matrix I should get When I request a travel time matrix I should get
| | a | b | | | a | b |
| a | 0 | 5 | | a | 0 | 5 |
| b | 5 | 0 | | b | 5 | 0 |

4
include/engine/api/table_parameters.hpp
View File

@ -59,7 +59,7 @@ struct TableParameters : public BaseParameters
{ {
std::vector<std::size_t> sources; std::vector<std::size_t> sources;
std::vector<std::size_t> destinations; std::vector<std::size_t> destinations;
double fallback_speed = 0; double fallback_speed = INVALID_FALLBACK_SPEED;
enum class FallbackCoordinateType enum class FallbackCoordinateType
{ {
@ -137,7 +137,7 @@ struct TableParameters : public BaseParameters
if (std::any_of(begin(destinations), end(destinations), not_in_range)) if (std::any_of(begin(destinations), end(destinations), not_in_range))
return false; return false;
if (fallback_speed < 0) if (fallback_speed <= 0)
return false; return false;
if (scale_factor <= 0) if (scale_factor <= 0)

2
include/nodejs/node_osrm_support.hpp
View File

@ -1192,7 +1192,7 @@ argumentsToTableParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
Nan::ThrowError("fallback_speed must be a number"); Nan::ThrowError("fallback_speed must be a number");
return table_parameters_ptr(); return table_parameters_ptr();
} }
else if (fallback_speed->NumberValue() < 0) else if (fallback_speed->NumberValue() <= 0)
{ {
Nan::ThrowError("fallback_speed must be > 0"); Nan::ThrowError("fallback_speed must be > 0");
return table_parameters_ptr(); return table_parameters_ptr();

1
include/util/typedefs.hpp
View File

@ -116,6 +116,7 @@ static const EdgeDuration MAXIMAL_EDGE_DURATION = std::numeric_limits<EdgeDurati
static const EdgeDistance MAXIMAL_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max(); static const EdgeDistance MAXIMAL_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max();
static const TurnPenalty INVALID_TURN_PENALTY = std::numeric_limits<TurnPenalty>::max(); static const TurnPenalty INVALID_TURN_PENALTY = std::numeric_limits<TurnPenalty>::max();
static const EdgeDistance INVALID_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max(); static const EdgeDistance INVALID_EDGE_DISTANCE = std::numeric_limits<EdgeDistance>::max();
static const EdgeDistance INVALID_FALLBACK_SPEED = std::numeric_limits<double>::max();
// FIXME the bitfields we use require a reduced maximal duration, this should be kept consistent // FIXME the bitfields we use require a reduced maximal duration, this should be kept consistent
// within the code base. For now we have to ensure that we don't case 30 bit to -1 and break any // within the code base. For now we have to ensure that we don't case 30 bit to -1 and break any

4
src/engine/plugins/table.cpp
View File

@ -96,7 +96,7 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
} }
// Scan table for null results - if any exist, replace with distance estimates // Scan table for null results - if any exist, replace with distance estimates
if (params.fallback_speed > 0 || params.scale_factor > 0) if (params.fallback_speed != INVALID_FALLBACK_SPEED || params.scale_factor > 0)
{ {
for (std::size_t row = 0; row < num_sources; row++) for (std::size_t row = 0; row < num_sources; row++)
{ {
@ -104,7 +104,7 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
{ {
const auto &table_index = row * num_destinations + column; const auto &table_index = row * num_destinations + column;
BOOST_ASSERT(table_index < result_tables_pair.first.size()); BOOST_ASSERT(table_index < result_tables_pair.first.size());
if (params.fallback_speed > 0 && if (params.fallback_speed != INVALID_FALLBACK_SPEED && params.fallback_speed > 0 &&
result_tables_pair.first[table_index] == MAXIMAL_EDGE_DURATION) result_tables_pair.first[table_index] == MAXIMAL_EDGE_DURATION)
{ {
const auto &source = const auto &source =

2
src/server/service/table_service.cpp
View File

@ -56,7 +56,7 @@ std::string getWrongOptionHelp(const engine::api::TableParameters &parameters)
help = "Number of coordinates needs to be at least two."; help = "Number of coordinates needs to be at least two.";
} }
if (parameters.fallback_speed < 0) if (parameters.fallback_speed <= 0)
{ {
help = "fallback_speed must be > 0"; help = "fallback_speed must be > 0";
} }

23
test/nodejs/table.js
View File

@ -260,6 +260,29 @@ tables.forEach(function(annotation) {
}); });
}); });
test('table: ' + annotation + ' table in Monaco with invalid fallback speeds and fallback coordinates', function(assert) {
assert.plan(4);
var osrm = new OSRM({path: mld_data_path, algorithm: 'MLD'});
var options = {
coordinates: two_test_coordinates,
annotations: [annotation.slice(0,-1)],
fallback_speed: -1
};
assert.throws(()=>osrm.table(options, (err, res) => {}), /fallback_speed must be > 0/, "should throw on invalid fallback_speeds");
options.fallback_speed = '10';
assert.throws(()=>osrm.table(options, (err, res) => {}), /fallback_speed must be a number/, "should throw on invalid fallback_speeds");
options.fallback_speed = 10;
options.fallback_coordinate = 'bla';
assert.throws(()=>osrm.table(options, (err, res) => {}), /fallback_coordinate' param must be one of \[input, snapped\]/, "should throw on invalid fallback_coordinate");
options.fallback_coordinate = 10;
assert.throws(()=>osrm.table(options, (err, res) => {}), /fallback_coordinate must be a string: \[input, snapped\]/, "should throw on invalid fallback_coordinate");
});
test('table: ' + annotation + ' table in Monaco with invalid scale factor', function(assert) { test('table: ' + annotation + ' table in Monaco with invalid scale factor', function(assert) {
assert.plan(3); assert.plan(3);
var osrm = new OSRM({path: mld_data_path, algorithm: 'MLD'}); var osrm = new OSRM({path: mld_data_path, algorithm: 'MLD'});

58
unit_tests/server/parameters_parser.cpp
View File

@ -91,10 +91,23 @@ BOOST_AUTO_TEST_CASE(invalid_table_urls)
49UL); 49UL);
BOOST_CHECK_EQUAL(testInvalidOptions<TableParameters>("1,2;3,4?fallback_coordinate=asdf"), BOOST_CHECK_EQUAL(testInvalidOptions<TableParameters>("1,2;3,4?fallback_coordinate=asdf"),
28UL); 28UL);
BOOST_CHECK_EQUAL(testInvalidOptions<TableParameters>("1,2;3,4?fallback_coordinate=10"), 28UL);
BOOST_CHECK_EQUAL( BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&scale_factor=-1"), 28UL); testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&scale_factor=-1"), 28UL);
BOOST_CHECK_EQUAL( BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&scale_factor=0"), 28UL); testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&scale_factor=0"), 28UL);
BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&fallback_speed=0"),
28UL);
BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&fallback_speed=-1"),
28UL);
BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&fallback_speed=0"),
28UL);
BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&fallback_speed=-1"),
28UL);
} }
BOOST_AUTO_TEST_CASE(valid_route_hint) BOOST_AUTO_TEST_CASE(valid_route_hint)
@ -570,16 +583,43 @@ BOOST_AUTO_TEST_CASE(valid_table_urls)
CHECK_EQUAL_RANGE(reference_7.sources, result_7->sources); CHECK_EQUAL_RANGE(reference_7.sources, result_7->sources);
CHECK_EQUAL_RANGE(reference_7.destinations, result_7->destinations); CHECK_EQUAL_RANGE(reference_7.destinations, result_7->destinations);
auto result_8 = parseParameters<TableParameters>("1,2;3,4?sources=all&destinations=all&" TableParameters reference_8{};
"annotations=duration&fallback_speed=1&" reference_8.coordinates = coords_1;
"fallback_coordinate=snapped&scale_factor=2"); auto result_8 =
parseParameters<TableParameters>("1,2;3,4?annotations=distance&fallback_speed=2.5");
BOOST_CHECK(result_8); BOOST_CHECK(result_8);
CHECK_EQUAL_RANGE(reference_1.sources, result_3->sources); BOOST_CHECK_EQUAL(result_8->annotations & TableParameters::AnnotationsType::Distance, true);
CHECK_EQUAL_RANGE(reference_1.destinations, result_3->destinations); CHECK_EQUAL_RANGE(reference_8.sources, result_8->sources);
CHECK_EQUAL_RANGE(reference_1.bearings, result_3->bearings); CHECK_EQUAL_RANGE(reference_8.destinations, result_8->destinations);
CHECK_EQUAL_RANGE(reference_1.radiuses, result_3->radiuses);
CHECK_EQUAL_RANGE(reference_1.approaches, result_3->approaches); TableParameters reference_9{};
CHECK_EQUAL_RANGE(reference_1.coordinates, result_3->coordinates); reference_9.coordinates = coords_1;
auto result_9 = parseParameters<TableParameters>(
"1,2;3,4?annotations=distance&fallback_speed=2.5&fallback_coordinate=input");
BOOST_CHECK(result_9);
BOOST_CHECK_EQUAL(result_9->annotations & TableParameters::AnnotationsType::Distance, true);
CHECK_EQUAL_RANGE(reference_9.sources, result_9->sources);
CHECK_EQUAL_RANGE(reference_9.destinations, result_9->destinations);
TableParameters reference_10{};
reference_10.coordinates = coords_1;
auto result_10 = parseParameters<TableParameters>(
"1,2;3,4?annotations=distance&fallback_speed=20&fallback_coordinate=snapped");
BOOST_CHECK(result_10);
BOOST_CHECK_EQUAL(result_10->annotations & TableParameters::AnnotationsType::Distance, true);
CHECK_EQUAL_RANGE(reference_10.sources, result_10->sources);
CHECK_EQUAL_RANGE(reference_10.destinations, result_10->destinations);
auto result_11 = parseParameters<TableParameters>("1,2;3,4?sources=all&destinations=all&"
"annotations=duration&fallback_speed=1&"
"fallback_coordinate=snapped&scale_factor=2");
BOOST_CHECK(result_11);
CHECK_EQUAL_RANGE(reference_1.sources, result_11->sources);
CHECK_EQUAL_RANGE(reference_1.destinations, result_11->destinations);
CHECK_EQUAL_RANGE(reference_1.bearings, result_11->bearings);
CHECK_EQUAL_RANGE(reference_1.radiuses, result_11->radiuses);
CHECK_EQUAL_RANGE(reference_1.approaches, result_11->approaches);
CHECK_EQUAL_RANGE(reference_1.coordinates, result_11->coordinates);
} }
BOOST_AUTO_TEST_CASE(valid_match_urls) BOOST_AUTO_TEST_CASE(valid_match_urls)