From 570d81c6dd3aa3e63f085dd6b404b53983eac360 Mon Sep 17 00:00:00 2001 From: Daniel Patterson Date: Fri, 19 May 2017 10:52:44 -0700 Subject: [PATCH] Bypass boost::numeric_cast checks for coordinate values that have already been validated (#4059) (i.e. stuff that's stored in our datafiles). Keep those checks for user-supplied values (i.e. coordinates coming from files during preprocessing, or coordinates supplied by users during requests) --- .../server/api/base_parameters_grammar.hpp | 18 +++---- include/util/coordinate.hpp | 52 ++++++++++++++++++- src/extractor/extractor_callbacks.cpp | 4 +- unit_tests/util/coordinate_calculation.cpp | 17 +++--- 4 files changed, 71 insertions(+), 20 deletions(-) diff --git a/include/server/api/base_parameters_grammar.hpp b/include/server/api/base_parameters_grammar.hpp index 816fd6bea..21b2f6b3a 100644 --- a/include/server/api/base_parameters_grammar.hpp +++ b/include/server/api/base_parameters_grammar.hpp @@ -111,15 +111,15 @@ struct BaseParametersGrammar : boost::spirit::qi::grammar qi::_1, qi::_2)]; - location_rule = - (double_ > qi::lit(',') > - double_)[qi::_val = ph::bind( - [](double lon, double lat) { - return util::Coordinate(util::toFixed(util::FloatLongitude{lon}), - util::toFixed(util::FloatLatitude{lat})); - }, - qi::_1, - qi::_2)]; + location_rule = (double_ > qi::lit(',') > + double_)[qi::_val = ph::bind( + [](double lon, double lat) { + return util::Coordinate( + util::toFixed(util::UnsafeFloatLongitude{lon}), + util::toFixed(util::UnsafeFloatLatitude{lat})); + }, + qi::_1, + qi::_2)]; polyline_rule = qi::as_string[qi::lit("polyline(") > +polyline_chars > ')'] [qi::_val = ph::bind( diff --git a/include/util/coordinate.hpp b/include/util/coordinate.hpp index 6ef431c8f..996466c32 100644 --- a/include/util/coordinate.hpp +++ b/include/util/coordinate.hpp @@ -53,16 +53,29 @@ struct latitude struct longitude { }; +struct unsafelatitude +{ +}; +struct unsafelongitude +{ +}; } +// Internal lon/lat types - assumed to be range safe using FixedLatitude = Alias; using FixedLongitude = Alias; using FloatLatitude = Alias; using FloatLongitude = Alias; +// Types used for external input data - conversion functions perform extra +// range checks on these (toFixed/toFloat, etc) +using UnsafeFloatLatitude = Alias; +using UnsafeFloatLongitude = Alias; static_assert(std::is_pod(), "FixedLatitude is not a valid alias"); static_assert(std::is_pod(), "FixedLongitude is not a valid alias"); static_assert(std::is_pod(), "FloatLatitude is not a valid alias"); static_assert(std::is_pod(), "FloatLongitude is not a valid alias"); +static_assert(std::is_pod(), "UnsafeFloatLatitude is not a valid alias"); +static_assert(std::is_pod(), "UnsafeFloatLongitude is not a valid alias"); /** * Converts a typed latitude from floating to fixed representation. @@ -72,6 +85,21 @@ static_assert(std::is_pod(), "FloatLongitude is not a valid alia * \see Coordinate, toFloating */ inline FixedLatitude toFixed(const FloatLatitude floating) +{ + const auto latitude = static_cast(floating); + const auto fixed = static_cast(std::round(latitude * COORDINATE_PRECISION)); + return FixedLatitude{fixed}; +} + +/** + * Converts a typed latitude from floating to fixed representation. Also performs an overflow check + * to ensure that the value fits inside the fixed representation. + * + * \param floating typed latitude in floating representation. + * \return typed latitude in fixed representation + * \see Coordinate, toFloating + */ +inline FixedLatitude toFixed(const UnsafeFloatLatitude floating) { const auto latitude = static_cast(floating); const auto fixed = @@ -87,6 +115,21 @@ inline FixedLatitude toFixed(const FloatLatitude floating) * \see Coordinate, toFloating */ inline FixedLongitude toFixed(const FloatLongitude floating) +{ + const auto longitude = static_cast(floating); + const auto fixed = static_cast(std::round(longitude * COORDINATE_PRECISION)); + return FixedLongitude{fixed}; +} + +/** + * Converts a typed longitude from floating to fixed representation. Also performns an overflow + * check to ensure that the value fits inside the fixed representation. + * + * \param floating typed longitude in floating representation. + * \return typed latitude in fixed representation + * \see Coordinate, toFloating + */ +inline FixedLongitude toFixed(const UnsafeFloatLongitude floating) { const auto longitude = static_cast(floating); const auto fixed = @@ -104,7 +147,7 @@ inline FixedLongitude toFixed(const FloatLongitude floating) inline FloatLatitude toFloating(const FixedLatitude fixed) { const auto latitude = static_cast(fixed); - const auto floating = boost::numeric_cast(latitude / COORDINATE_PRECISION); + const auto floating = static_cast(latitude) / COORDINATE_PRECISION; return FloatLatitude{floating}; } @@ -118,7 +161,7 @@ inline FloatLatitude toFloating(const FixedLatitude fixed) inline FloatLongitude toFloating(const FixedLongitude fixed) { const auto longitude = static_cast(fixed); - const auto floating = boost::numeric_cast(longitude / COORDINATE_PRECISION); + const auto floating = static_cast(longitude) / COORDINATE_PRECISION; return FloatLongitude{floating}; } @@ -152,6 +195,11 @@ struct Coordinate { } + Coordinate(const UnsafeFloatLongitude lon_, const UnsafeFloatLatitude lat_) + : Coordinate(toFixed(lon_), toFixed(lat_)) + { + } + Coordinate(const FixedLongitude lon_, const FixedLatitude lat_) : lon(lon_), lat(lat_) {} template Coordinate(const T &coordinate) : lon(coordinate.lon), lat(coordinate.lat) diff --git a/src/extractor/extractor_callbacks.cpp b/src/extractor/extractor_callbacks.cpp index 93f0bac7e..0e0d36cfb 100644 --- a/src/extractor/extractor_callbacks.cpp +++ b/src/extractor/extractor_callbacks.cpp @@ -54,8 +54,8 @@ void ExtractorCallbacks::ProcessNode(const osmium::Node &input_node, const ExtractionNode &result_node) { external_memory.all_nodes_list.push_back( - {util::toFixed(util::FloatLongitude{input_node.location().lon()}), - util::toFixed(util::FloatLatitude{input_node.location().lat()}), + {util::toFixed(util::UnsafeFloatLongitude{input_node.location().lon()}), + util::toFixed(util::UnsafeFloatLatitude{input_node.location().lat()}), OSMNodeID{static_cast(input_node.id())}, result_node.barrier, result_node.traffic_lights}); diff --git a/unit_tests/util/coordinate_calculation.cpp b/unit_tests/util/coordinate_calculation.cpp index 2abdda8ab..5dcf71c3a 100644 --- a/unit_tests/util/coordinate_calculation.cpp +++ b/unit_tests/util/coordinate_calculation.cpp @@ -125,14 +125,17 @@ BOOST_AUTO_TEST_CASE(compute_angle) end = Coordinate{FloatLongitude{1 + std::numeric_limits::epsilon()}, FloatLatitude{0}}; angle = coordinate_calculation::computeAngle(first, middle, end); BOOST_CHECK_EQUAL(angle, 180); +} - // Invalid values - BOOST_CHECK_THROW( - coordinate_calculation::computeAngle( - Coordinate(FloatLongitude{0}, FloatLatitude{0}), - Coordinate(FloatLongitude{1}, FloatLatitude{0}), - Coordinate(FloatLongitude{std::numeric_limits::max()}, FloatLatitude{0})), - boost::numeric::positive_overflow); +BOOST_AUTO_TEST_CASE(invalid_values) +{ + // Invalid values for unsafe types + BOOST_CHECK_THROW(coordinate_calculation::computeAngle( + Coordinate(UnsafeFloatLongitude{0}, UnsafeFloatLatitude{0}), + Coordinate(UnsafeFloatLongitude{1}, UnsafeFloatLatitude{0}), + Coordinate(UnsafeFloatLongitude{std::numeric_limits::max()}, + UnsafeFloatLatitude{0})), + boost::numeric::positive_overflow); } // Regression test for bug captured in #1347