diff --git a/include/nodejs/node_osrm_support.hpp b/include/nodejs/node_osrm_support.hpp index ba79a3b3d..a76b5ccf5 100644 --- a/include/nodejs/node_osrm_support.hpp +++ b/include/nodejs/node_osrm_support.hpp @@ -996,6 +996,18 @@ argumentsToRouteParameter(const Nan::FunctionCallbackInfo &args, } params->waypoints.emplace_back(static_cast(waypoint_value->NumberValue())); } + + if (!params->waypoints.empty()) + { + for (std::size_t i = 0; i < params->waypoints.size() - 1; i++) + { + if (params->waypoints[i] >= params->waypoints[i + 1]) + { + Nan::ThrowError("Waypoints must be supplied in increasing order"); + return route_parameters_ptr(); + } + } + } } bool parsedSuccessfully = parseCommonParameters(obj, params); diff --git a/src/engine/plugins/viaroute.cpp b/src/engine/plugins/viaroute.cpp index 9d16a56c6..a4df2de86 100644 --- a/src/engine/plugins/viaroute.cpp +++ b/src/engine/plugins/viaroute.cpp @@ -83,18 +83,6 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm json_result); } - if (!route_parameters.waypoints.empty()) - { - for (std::size_t i = 0; i < route_parameters.waypoints.size() - 1; i++) - { - if (route_parameters.waypoints[i] >= route_parameters.waypoints[i + 1]) - { - return Error( - "InvalidValue", "Waypoints must be supplied in increasing order", json_result); - } - } - } - if (!CheckAlgorithms(route_parameters, algorithms, json_result)) return Status::Error; @@ -157,20 +145,17 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm auto collapse_legs = !route_parameters.waypoints.empty(); if (collapse_legs) { - std::vector waypoint_legs; - auto waypoints_itr = route_parameters.waypoints.begin(); - for (std::size_t i = 0; i < route_parameters.coordinates.size(); i++) - { - if (i == *waypoints_itr) - { - waypoint_legs.push_back(true); - waypoints_itr++; - } - else - { - waypoint_legs.push_back(false); - } - } + std::vector waypoint_legs(route_parameters.coordinates.size(), false); + std::for_each(route_parameters.waypoints.begin(), + route_parameters.waypoints.end(), + [&](const std::size_t waypoint_index) { + BOOST_ASSERT(waypoint_index < waypoint_legs.size()); + waypoint_legs[waypoint_index] = true; + }); + // First and last coordinates should always be waypoints + // This gets validated earlier, but double-checking here, jic + BOOST_ASSERT(waypoint_legs.front()); + BOOST_ASSERT(waypoint_legs.back()); for (std::size_t i = 0; i < routes.routes.size(); i++) { routes.routes[i] = CollapseInternalRouteResult(routes.routes[i], waypoint_legs); diff --git a/test/nodejs/route.js b/test/nodejs/route.js index bef370d75..037a7be10 100644 --- a/test/nodejs/route.js +++ b/test/nodejs/route.js @@ -677,4 +677,16 @@ test('route: throws on invalid waypoints values, waypoints must be an array of i }; assert.throws(function () { osrm.route(options, function (err, response) { console.log(err); }); }, 'Waypoint values must be an array of integers'); +}); + +test('route: throws on invalid waypoints values, waypoints must be an array of integers in increasing order', function (assert) { + assert.plan(1); + var osrm = new OSRM(monaco_path); + var options = { + steps: true, + coordinates: three_test_coordinates.concat(three_test_coordinates), + waypoints: [0,2,1,5] + }; + assert.throws(function () { osrm.route(options, function (err, response) { console.error(`response: ${response}`); console.error(`error: ${err}`); }); }, + /Waypoints must be supplied in increasing order/); }); \ No newline at end of file