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

Validate source/destination indices correctly in nodejs support (#5595)

Browse Source 
* validate source/destination indices correctly

Co-authored-by: Denis Chapligin <denis.chaplygin@wolt.com>
Co-authored-by: Daniel Patterson <danpat@danpat.net>
This commit is contained in:
Karen Shea 2021-01-28 15:02:01 +01:00 committed by GitHub
parent 960269f95a
commit a613375460
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 22 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGELOG.md
View File

@ -10,6 +10,7 @@
- Misc:
- CHANGED: Cleanup NodeJS dependencies [#5945](https://github.com/Project-OSRM/osrm-backend/pull/5945)
- CHANGED: Unify `.osrm.turn_penalites_index` dump processing same with `.osrm.turn_weight_penalties` and `.osrm.turn_duration_penalties` [#5868](https://github.com/Project-OSRM/osrm-backend/pull/5868)
- FIXED: Properly validate source/destination validation in NodeJS table service [#5595](https://github.com/Project-OSRM/osrm-backend/pull/5595/files)
- FIXED: turn.roads_on_the_left not containing incoming roads and turn.roads_on_the_right not containing outgoing roads on two-way roads [#5128](https://github.com/Project-OSRM/osrm-backend/issues/5128)
- Profile:
- ADDED: Profile debug script which fetches a way from OSM then outputs the result of the profile. [#5908](https://github.com/Project-OSRM/osrm-backend/pull/5908)
@ -20,6 +21,7 @@
- FIXED: Fix vector bool permutation in graph contraction step [#5882](https://github.com/Project-OSRM/osrm-backend/pull/5882)
- API:
- FIXED: Undo libosrm API break by adding old interface as method overload [#5861](https://github.com/Project-OSRM/osrm-backend/pull/5861)
- FIXED: Fixed validation of sources/destinations when accessed via node bindings [#5595](https://github.com/Project-OSRM/osrm-backend/pull/5595)
# 5.23.0
- Changes from 5.22.0

9
include/nodejs/node_osrm_support.hpp
View File

@ -1210,10 +1210,9 @@ argumentsToTableParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
if (source->IsUint32())
{
size_t source_value = Nan::To<unsigned>(source).FromJust();
if (source_value > params->coordinates.size())
if (source_value >= params->coordinates.size())
{
Nan::ThrowError(
"Source indices must be less than or equal to the number of coordinates");
Nan::ThrowError("Source indices must be less than the number of coordinates");
return table_parameters_ptr();
}
@ -1250,9 +1249,9 @@ argumentsToTableParameter(const Nan::FunctionCallbackInfo<v8::Value> &args,
if (destination->IsUint32())
{
size_t destination_value = Nan::To<unsigned>(destination).FromJust();
if (destination_value > params->coordinates.size())
if (destination_value >= params->coordinates.size())
{
Nan::ThrowError("Destination indices must be less than or equal to the number "
Nan::ThrowError("Destination indices must be less than the number "
"of coordinates");
return table_parameters_ptr();
}

12
test/nodejs/table.js
View File

@ -130,7 +130,7 @@ tables.forEach(function(annotation) {
});
test('table: ' + annotation + ' throws on invalid arguments', function(assert) {
assert.plan(15);
assert.plan(17);
var osrm = new OSRM(data_path);
var options = {annotations: [annotation.slice(0,-1)]};
assert.throws(function() { osrm.table(options); },
@ -157,10 +157,13 @@ tables.forEach(function(annotation) {
/Sources must be an array of indices \(or undefined\)/);
options.sources = [0, 4];
assert.throws(function() { osrm.table(options, function(err, response) {}) },
/Source indices must be less than or equal to the number of coordinates/);
/Source indices must be less than the number of coordinates/);
options.sources = [0.3, 1.1];
assert.throws(function() { osrm.table(options, function(err, response) {}) },
/Source must be an integer/);
options.sources = [0, 1, 2];
assert.throws(function() { osrm.table(options, function(err, response) {}) },
/Source indices must be less than the number of coordinates/);
options.destinations = true;
delete options.sources;
@ -168,10 +171,13 @@ tables.forEach(function(annotation) {
/Destinations must be an array of indices \(or undefined\)/);
options.destinations = [0, 4];
assert.throws(function() { osrm.table(options, function(err, response) {}) },
/Destination indices must be less than or equal to the number of coordinates/);
/Destination indices must be less than the number of coordinates/);
options.destinations = [0.3, 1.1];
assert.throws(function() { osrm.table(options, function(err, response) {}) },
/Destination must be an integer/);
options.destinations = [0, 4];
assert.throws(function() { osrm.table(options, function(err, response) {}) },
/Destination indices must be less than the number of coordinates/);
// does not throw: the following two have been changed in OSRM v5
options.sources = [0, 1];

7
unit_tests/server/parameters_parser.cpp
View File

@ -108,6 +108,13 @@ BOOST_AUTO_TEST_CASE(invalid_table_urls)
BOOST_CHECK_EQUAL(
testInvalidOptions<TableParameters>("1,2;3,4?annotations=durations&fallback_speed=-1"),
28UL);
// TODO(danpat): this is only testing invalid grammar which isn't capable of checking
// for values that need to reference other things currently. These
// requests are gramatically correct, but semantically incorrect.
// The table service properly fails these, as it checks IsValid() after
// parsing, which fails when sources/destinations are too large
// BOOST_CHECK_EQUAL(testInvalidOptions<TableParameters>("1,2;3,4?sources=2"), 7UL);
// BOOST_CHECK_EQUAL(testInvalidOptions<TableParameters>("1,2;3,4?destinations=2"), 7UL);
}
BOOST_AUTO_TEST_CASE(valid_route_hint)