From 5186b9490d458060f1455f4454f111ab2ef3829b Mon Sep 17 00:00:00 2001 From: Michael Krasnyk Date: Tue, 26 Apr 2016 21:20:24 +0200 Subject: [PATCH] Fix syntax error position indicators in parameters queries To fix #2193 prefix_length member variable has been added to ParsedURL that is set to the length of "/service/version/profile/" prefix when the prefix is accepted by the parser. Also BOOST_FUSION_ADAPT_STRUCT for osrm::server::api::ParsedURL has been moved from header to url_parser.cpp to speed up compilation of CUs that do not use the fusion adaption. --- features/testbot/status.feature | 18 ++++++------- include/server/api/parsed_url.hpp | 10 +------- include/server/service/base_service.hpp | 2 +- include/server/service/match_service.hpp | 2 +- include/server/service/nearest_service.hpp | 2 +- include/server/service/route_service.hpp | 2 +- include/server/service/table_service.hpp | 2 +- include/server/service/tile_service.hpp | 2 +- include/server/service/trip_service.hpp | 2 +- src/server/api/url_parser.cpp | 30 +++++++++++++++++----- src/server/service/match_service.cpp | 4 +-- src/server/service/nearest_service.cpp | 4 +-- src/server/service/route_service.cpp | 4 +-- src/server/service/table_service.cpp | 4 +-- src/server/service/tile_service.cpp | 4 +-- src/server/service/trip_service.cpp | 4 +-- src/server/service_handler.cpp | 2 +- unit_tests/server/url_parser.cpp | 17 +++++++----- 18 files changed, 64 insertions(+), 51 deletions(-) diff --git a/features/testbot/status.feature b/features/testbot/status.feature index 2221461a6..8da2b155b 100644 --- a/features/testbot/status.feature +++ b/features/testbot/status.feature @@ -55,13 +55,13 @@ Feature: Status messages | ? | 400 | URL string malformed close to position 1: "/?" | | route/v1/driving | 400 | URL string malformed close to position 17: "ing" | | route/v1/driving/ | 400 | URL string malformed close to position 18: "ng/" | - | route/v1/driving/1 | 400 | Query string malformed close to position 1 | + | route/v1/driving/1 | 400 | Query string malformed close to position 19 | | route/v1/driving/1,1 | 400 | Number of coordinates needs to be at least two. | - | route/v1/driving/1,1,1 | 400 | Query string malformed close to position 3 | - | route/v1/driving/x | 400 | Query string malformed close to position 0 | - | route/v1/driving/x,y | 400 | Query string malformed close to position 0 | - | route/v1/driving/1,1; | 400 | Query string malformed close to position 3 | - | route/v1/driving/1,1;1 | 400 | Query string malformed close to position 5 | - | route/v1/driving/1,1;1,1,1 | 400 | Query string malformed close to position 7 | - | route/v1/driving/1,1;x | 400 | Query string malformed close to position 3 | - | route/v1/driving/1,1;x,y | 400 | Query string malformed close to position 3 | + | route/v1/driving/1,1,1 | 400 | Query string malformed close to position 21 | + | route/v1/driving/x | 400 | Query string malformed close to position 18 | + | route/v1/driving/x,y | 400 | Query string malformed close to position 18 | + | route/v1/driving/1,1; | 400 | Query string malformed close to position 21 | + | route/v1/driving/1,1;1 | 400 | Query string malformed close to position 23 | + | route/v1/driving/1,1;1,1,1 | 400 | Query string malformed close to position 25 | + | route/v1/driving/1,1;x | 400 | Query string malformed close to position 21 | + | route/v1/driving/1,1;x,y | 400 | Query string malformed close to position 21 | diff --git a/include/server/api/parsed_url.hpp b/include/server/api/parsed_url.hpp index c80519636..0fefc65db 100644 --- a/include/server/api/parsed_url.hpp +++ b/include/server/api/parsed_url.hpp @@ -3,8 +3,6 @@ #include "util/coordinate.hpp" -#include - #include #include @@ -21,17 +19,11 @@ struct ParsedURL final unsigned version; std::string profile; std::string query; + std::size_t prefix_length; }; } // api } // server } // osrm -BOOST_FUSION_ADAPT_STRUCT(osrm::server::api::ParsedURL, - (std::string, service) - (unsigned, version) - (std::string, profile) - (std::string, query) -) - #endif diff --git a/include/server/service/base_service.hpp b/include/server/service/base_service.hpp index 36c82dbf9..cfa8581a4 100644 --- a/include/server/service/base_service.hpp +++ b/include/server/service/base_service.hpp @@ -25,7 +25,7 @@ class BaseService BaseService(OSRM &routing_machine) : routing_machine(routing_machine) {} virtual ~BaseService() = default; - virtual engine::Status RunQuery(std::string &query, ResultT &result) = 0; + virtual engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) = 0; virtual unsigned GetVersion() = 0; diff --git a/include/server/service/match_service.hpp b/include/server/service/match_service.hpp index 23212c182..a0aed941a 100644 --- a/include/server/service/match_service.hpp +++ b/include/server/service/match_service.hpp @@ -22,7 +22,7 @@ class MatchService final : public BaseService public: MatchService(OSRM &routing_machine) : BaseService(routing_machine) {} - engine::Status RunQuery(std::string &query, ResultT &result) final override; + engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) final override; unsigned GetVersion() final override { return 1; } }; diff --git a/include/server/service/nearest_service.hpp b/include/server/service/nearest_service.hpp index 222003c7f..62fc1b129 100644 --- a/include/server/service/nearest_service.hpp +++ b/include/server/service/nearest_service.hpp @@ -22,7 +22,7 @@ class NearestService final : public BaseService public: NearestService(OSRM &routing_machine) : BaseService(routing_machine) {} - engine::Status RunQuery(std::string &query, ResultT &result) final override; + engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) final override; unsigned GetVersion() final override { return 1; } }; diff --git a/include/server/service/route_service.hpp b/include/server/service/route_service.hpp index 11155856e..639aea210 100644 --- a/include/server/service/route_service.hpp +++ b/include/server/service/route_service.hpp @@ -22,7 +22,7 @@ class RouteService final : public BaseService public: RouteService(OSRM &routing_machine) : BaseService(routing_machine) {} - engine::Status RunQuery(std::string &query, ResultT &result) final override; + engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) final override; unsigned GetVersion() final override { return 1; } }; diff --git a/include/server/service/table_service.hpp b/include/server/service/table_service.hpp index 61b9d84b1..7839bd40e 100644 --- a/include/server/service/table_service.hpp +++ b/include/server/service/table_service.hpp @@ -22,7 +22,7 @@ class TableService final : public BaseService public: TableService(OSRM &routing_machine) : BaseService(routing_machine) {} - engine::Status RunQuery(std::string &query, ResultT &result) final override; + engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) final override; unsigned GetVersion() final override { return 1; } }; diff --git a/include/server/service/tile_service.hpp b/include/server/service/tile_service.hpp index 05365d437..d984a0bdd 100644 --- a/include/server/service/tile_service.hpp +++ b/include/server/service/tile_service.hpp @@ -22,7 +22,7 @@ class TileService final : public BaseService public: TileService(OSRM &routing_machine) : BaseService(routing_machine) {} - engine::Status RunQuery(std::string &query, ResultT &result) final override; + engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) final override; unsigned GetVersion() final override { return 1; } }; diff --git a/include/server/service/trip_service.hpp b/include/server/service/trip_service.hpp index 7daeba6f0..d8dfed4b7 100644 --- a/include/server/service/trip_service.hpp +++ b/include/server/service/trip_service.hpp @@ -22,7 +22,7 @@ class TripService final : public BaseService public: TripService(OSRM &routing_machine) : BaseService(routing_machine) {} - engine::Status RunQuery(std::string &query, ResultT &result) final override; + engine::Status RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) final override; unsigned GetVersion() final override { return 1; } }; diff --git a/src/server/api/url_parser.cpp b/src/server/api/url_parser.cpp index 7ef6db001..875e78abc 100644 --- a/src/server/api/url_parser.cpp +++ b/src/server/api/url_parser.cpp @@ -1,15 +1,25 @@ #include "server/api/url_parser.hpp" #include "engine/polyline_compressor.hpp" -//#define BOOST_SPIRIT_DEBUG +#include +#include #include +#include #include #include +BOOST_FUSION_ADAPT_STRUCT(osrm::server::api::ParsedURL, + (std::string, service) + (unsigned, version) + (std::string, profile) + (std::string, query) +) + // Keep impl. TU local namespace { +namespace ph = boost::phoenix; namespace qi = boost::spirit::qi; template // @@ -17,6 +27,8 @@ struct URLParser final : qi::grammar { URLParser() : URLParser::base_type(start) { + using boost::spirit::repository::qi::iter_pos; + alpha_numeral = qi::char_("a-zA-Z0-9"); polyline_chars = qi::char_("a-zA-Z0-9_.--[]{}@?|\\%~`^"); all_chars = polyline_chars | qi::char_("=,;:&()."); @@ -28,10 +40,14 @@ struct URLParser final : qi::grammar // Example input: /route/v1/driving/7.416351,43.731205;7.420363,43.736189 - start = qi::lit('/') > service // - > qi::lit('/') > qi::lit('v') > version // - > qi::lit('/') > profile // - > qi::lit('/') > query; // + start + = qi::lit('/') > service + > qi::lit('/') > qi::lit('v') > version + > qi::lit('/') > profile + > qi::lit('/') + > qi::omit[iter_pos[ph::bind(&osrm::server::api::ParsedURL::prefix_length, qi::_val) = qi::_1 - qi::_r1]] + > query + ; BOOST_SPIRIT_DEBUG_NODES((start)(service)(version)(profile)(query)) } @@ -61,12 +77,12 @@ boost::optional parseURL(std::string::iterator &iter, const std::stri { using It = std::decay::type; - static URLParser const parser; + static URLParser const parser; ParsedURL out; try { - const auto ok = boost::spirit::qi::parse(iter, end, parser, out); + const auto ok = boost::spirit::qi::parse(iter, end, parser(boost::phoenix::val(iter)), out); if (ok && iter == end) return boost::make_optional(out); diff --git a/src/server/service/match_service.cpp b/src/server/service/match_service.cpp index dec8006e1..7321b8680 100644 --- a/src/server/service/match_service.cpp +++ b/src/server/service/match_service.cpp @@ -40,7 +40,7 @@ std::string getWrongOptionHelp(const engine::api::MatchParameters ¶meters) } } // anon. ns -engine::Status MatchService::RunQuery(std::string &query, ResultT &result) +engine::Status MatchService::RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) { result = util::json::Object(); auto &json_result = result.get(); @@ -53,7 +53,7 @@ engine::Status MatchService::RunQuery(std::string &query, ResultT &result) const auto position = std::distance(query.begin(), query_iterator); json_result.values["code"] = "InvalidQuery"; json_result.values["message"] = - "Query string malformed close to position " + std::to_string(position); + "Query string malformed close to position " + std::to_string(prefix_length + position); return engine::Status::Error; } diff --git a/src/server/service/nearest_service.cpp b/src/server/service/nearest_service.cpp index 1ea1fa89b..4ffd021ff 100644 --- a/src/server/service/nearest_service.cpp +++ b/src/server/service/nearest_service.cpp @@ -39,7 +39,7 @@ std::string getWrongOptionHelp(const engine::api::NearestParameters ¶meters) } } // anon. ns -engine::Status NearestService::RunQuery(std::string &query, ResultT &result) +engine::Status NearestService::RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) { result = util::json::Object(); auto &json_result = result.get(); @@ -52,7 +52,7 @@ engine::Status NearestService::RunQuery(std::string &query, ResultT &result) const auto position = std::distance(query.begin(), query_iterator); json_result.values["code"] = "InvalidQuery"; json_result.values["message"] = - "Query string malformed close to position " + std::to_string(position); + "Query string malformed close to position " + std::to_string(prefix_length + position); return engine::Status::Error; } BOOST_ASSERT(parameters); diff --git a/src/server/service/route_service.cpp b/src/server/service/route_service.cpp index f06cbdbd3..a7b85b574 100644 --- a/src/server/service/route_service.cpp +++ b/src/server/service/route_service.cpp @@ -36,7 +36,7 @@ std::string getWrongOptionHelp(const engine::api::RouteParameters ¶meters) } } // anon. ns -engine::Status RouteService::RunQuery(std::string &query, ResultT &result) +engine::Status RouteService::RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) { result = util::json::Object(); auto &json_result = result.get(); @@ -49,7 +49,7 @@ engine::Status RouteService::RunQuery(std::string &query, ResultT &result) const auto position = std::distance(query.begin(), query_iterator); json_result.values["code"] = "InvalidQuery"; json_result.values["message"] = - "Query string malformed close to position " + std::to_string(position); + "Query string malformed close to position " + std::to_string(prefix_length + position); return engine::Status::Error; } BOOST_ASSERT(parameters); diff --git a/src/server/service/table_service.cpp b/src/server/service/table_service.cpp index 9b0f817b8..bb930191f 100644 --- a/src/server/service/table_service.cpp +++ b/src/server/service/table_service.cpp @@ -57,7 +57,7 @@ std::string getWrongOptionHelp(const engine::api::TableParameters ¶meters) } } // anon. ns -engine::Status TableService::RunQuery(std::string &query, ResultT &result) +engine::Status TableService::RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) { result = util::json::Object(); auto &json_result = result.get(); @@ -70,7 +70,7 @@ engine::Status TableService::RunQuery(std::string &query, ResultT &result) const auto position = std::distance(query.begin(), query_iterator); json_result.values["code"] = "InvalidQuery"; json_result.values["message"] = - "Query string malformed close to position " + std::to_string(position); + "Query string malformed close to position " + std::to_string(prefix_length + position); return engine::Status::Error; } BOOST_ASSERT(parameters); diff --git a/src/server/service/tile_service.cpp b/src/server/service/tile_service.cpp index d8f1ba8a5..a4ded2cfe 100644 --- a/src/server/service/tile_service.cpp +++ b/src/server/service/tile_service.cpp @@ -15,7 +15,7 @@ namespace server namespace service { -engine::Status TileService::RunQuery(std::string &query, ResultT &result) +engine::Status TileService::RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) { auto query_iterator = query.begin(); auto parameters = @@ -27,7 +27,7 @@ engine::Status TileService::RunQuery(std::string &query, ResultT &result) auto &json_result = result.get(); json_result.values["code"] = "InvalidQuery"; json_result.values["message"] = - "Query string malformed close to position " + std::to_string(position); + "Query string malformed close to position " + std::to_string(prefix_length + position); return engine::Status::Error; } BOOST_ASSERT(parameters); diff --git a/src/server/service/trip_service.cpp b/src/server/service/trip_service.cpp index 82a1bfdd2..e0b51c08e 100644 --- a/src/server/service/trip_service.cpp +++ b/src/server/service/trip_service.cpp @@ -38,7 +38,7 @@ std::string getWrongOptionHelp(const engine::api::TripParameters ¶meters) } } // anon. ns -engine::Status TripService::RunQuery(std::string &query, ResultT &result) +engine::Status TripService::RunQuery(std::size_t prefix_length, std::string &query, ResultT &result) { result = util::json::Object(); auto &json_result = result.get(); @@ -53,7 +53,7 @@ engine::Status TripService::RunQuery(std::string &query, ResultT &result) auto &json_result = result.get(); json_result.values["code"] = "InvalidQuery"; json_result.values["message"] = - "Query string malformed close to position " + std::to_string(position); + "Query string malformed close to position " + std::to_string(prefix_length + position); return engine::Status::Error; } BOOST_ASSERT(parameters); diff --git a/src/server/service_handler.cpp b/src/server/service_handler.cpp index 98ba6a617..03bcf86c4 100644 --- a/src/server/service_handler.cpp +++ b/src/server/service_handler.cpp @@ -48,7 +48,7 @@ engine::Status ServiceHandler::RunQuery(api::ParsedURL parsed_url, return engine::Status::Error; } - return service->RunQuery(parsed_url.query, result); + return service->RunQuery(parsed_url.prefix_length, parsed_url.query, result); } } } diff --git a/unit_tests/server/url_parser.cpp b/unit_tests/server/url_parser.cpp index cca1f6103..fd1f1103b 100644 --- a/unit_tests/server/url_parser.cpp +++ b/unit_tests/server/url_parser.cpp @@ -51,52 +51,57 @@ BOOST_AUTO_TEST_CASE(invalid_urls) BOOST_AUTO_TEST_CASE(valid_urls) { - api::ParsedURL reference_1{"route", 1, "profile", "0,1;2,3;4,5?options=value&foo=bar"}; + api::ParsedURL reference_1{"route", 1, "profile", "0,1;2,3;4,5?options=value&foo=bar", 18UL}; auto result_1 = api::parseURL("/route/v1/profile/0,1;2,3;4,5?options=value&foo=bar"); BOOST_CHECK(result_1); BOOST_CHECK_EQUAL(reference_1.service, result_1->service); BOOST_CHECK_EQUAL(reference_1.version, result_1->version); BOOST_CHECK_EQUAL(reference_1.profile, result_1->profile); CHECK_EQUAL_RANGE(reference_1.query, result_1->query); + BOOST_CHECK_EQUAL(reference_1.prefix_length, result_1->prefix_length); // no options - api::ParsedURL reference_2{"route", 1, "profile", "0,1;2,3;4,5"}; + api::ParsedURL reference_2{"route", 1, "profile", "0,1;2,3;4,5", 18UL}; auto result_2 = api::parseURL("/route/v1/profile/0,1;2,3;4,5"); BOOST_CHECK(result_2); BOOST_CHECK_EQUAL(reference_2.service, result_2->service); BOOST_CHECK_EQUAL(reference_2.version, result_2->version); BOOST_CHECK_EQUAL(reference_2.profile, result_2->profile); CHECK_EQUAL_RANGE(reference_2.query, result_2->query); + BOOST_CHECK_EQUAL(reference_2.prefix_length, result_2->prefix_length); // one coordinate std::vector coords_3 = { util::Coordinate(util::FloatLongitude(0), util::FloatLatitude(1)), }; - api::ParsedURL reference_3{"route", 1, "profile", "0,1"}; + api::ParsedURL reference_3{"route", 1, "profile", "0,1", 18UL}; auto result_3 = api::parseURL("/route/v1/profile/0,1"); BOOST_CHECK(result_3); BOOST_CHECK_EQUAL(reference_3.service, result_3->service); BOOST_CHECK_EQUAL(reference_3.version, result_3->version); BOOST_CHECK_EQUAL(reference_3.profile, result_3->profile); CHECK_EQUAL_RANGE(reference_3.query, result_3->query); + BOOST_CHECK_EQUAL(reference_3.prefix_length, result_3->prefix_length); // polyline - api::ParsedURL reference_5{"route", 1, "profile", "polyline(_ibE?_seK_seK_seK_seK)?"}; + api::ParsedURL reference_5{"route", 1, "profile", "polyline(_ibE?_seK_seK_seK_seK)?", 18UL}; auto result_5 = api::parseURL("/route/v1/profile/polyline(_ibE?_seK_seK_seK_seK)?"); BOOST_CHECK(result_5); BOOST_CHECK_EQUAL(reference_5.service, result_5->service); BOOST_CHECK_EQUAL(reference_5.version, result_5->version); BOOST_CHECK_EQUAL(reference_5.profile, result_5->profile); CHECK_EQUAL_RANGE(reference_5.query, result_5->query); + BOOST_CHECK_EQUAL(reference_5.prefix_length, result_5->prefix_length); // tile - api::ParsedURL reference_6{"route", 1, "profile", "tile(1,2,3).mvt"}; + api::ParsedURL reference_6{"route", 1, "profile", "tile(1,2,3).mvt", 18UL}; auto result_6 = api::parseURL("/route/v1/profile/tile(1,2,3).mvt"); - BOOST_CHECK(result_5); + BOOST_CHECK(result_6); BOOST_CHECK_EQUAL(reference_6.service, result_6->service); BOOST_CHECK_EQUAL(reference_6.version, result_6->version); BOOST_CHECK_EQUAL(reference_6.profile, result_6->profile); CHECK_EQUAL_RANGE(reference_6.query, result_6->query); + BOOST_CHECK_EQUAL(reference_6.prefix_length, result_6->prefix_length); } BOOST_AUTO_TEST_SUITE_END()