Decoding should not fail for incomplete polyline strings (#3404)

Possible fails in
1) correct lattitude, longitude is missing
2) no end-of-number (0 5th bit) marker in the last character

src/engine/polyline_compressor.cpp

@@ -2,6 +2,7 @@
 
 #include <algorithm>
 #include <boost/assert.hpp>
+#include <climits>
 #include <cmath>
 #include <cstddef>
 #include <cstdlib>
@@ -55,42 +56,50 @@ std::string encode(std::vector<int> &numbers)
 
 std::vector<util::Coordinate> decodePolyline(const std::string &geometry_string)
 {
-    std::vector<util::Coordinate> new_coordinates;
+    // https://developers.google.com/maps/documentation/utilities/polylinealgorithm
-    int index = 0, len = geometry_string.size();
+    auto decode_polyline_integer = [](auto &first, auto last) {
-    int lat = 0, lng = 0;
+        // varint coding parameters
+        const std::uint32_t bits_in_chunk = 5;
+        const std::uint32_t continuation_bit = 1 << bits_in_chunk;
+        const std::uint32_t chunk_mask = (1 << bits_in_chunk) - 1;
 
-    while (index < len)
+        std::uint32_t result = 0;
+        for (std::uint32_t value = continuation_bit, shift = 0;
+             (value & continuation_bit) && (shift < CHAR_BIT * sizeof(result) - 1) && first != last;
+             ++first, shift += bits_in_chunk)
+        {
+            value = *first - 63; // convert ASCII coding [?..~] to an integer [0..63]
+            result |= (value & chunk_mask) << shift;
+        }
+
+        // change "zig-zag" sign coding to two's complement
+        result = ((result & 1) == 1) ? ~(result >> 1) : (result >> 1);
+        return static_cast<std::int32_t>(result);
+    };
+
+    auto polyline_to_coordinate = [](auto value) {
+        return static_cast<std::int32_t>(value * detail::POLYLINE_TO_COORDINATE);
+    };
+
+    std::vector<util::Coordinate> coordinates;
+    std::int32_t latitude = 0, longitude = 0;
+
+    std::string::const_iterator first = geometry_string.begin();
+    const std::string::const_iterator last = geometry_string.end();
+    while (first != last)
     {
-        int b, shift = 0, result = 0;
+        const auto dlat = decode_polyline_integer(first, last);
-        do
+        const auto dlon = decode_polyline_integer(first, last);
-        {
-            b = geometry_string.at(index++) - 63;
-            result |= (b & 0x1f) << shift;
-            shift += 5;
-        } while (b >= 0x20 && index < len);
-        int dlat = ((result & 1) != 0 ? ~(result >> 1) : (result >> 1));
-        lat += dlat;
 
-        shift = 0;
+        latitude += dlat;
-        result = 0;
+        longitude += dlon;
-        do
-        {
-            b = geometry_string.at(index++) - 63;
-            result |= (b & 0x1f) << shift;
-            shift += 5;
-        } while (b >= 0x20 && index < len);
-        int dlng = ((result & 1) != 0 ? ~(result >> 1) : (result >> 1));
-        lng += dlng;
 
-        util::Coordinate p;
+        coordinates.emplace_back(
-        p.lat =
+            util::Coordinate{util::FixedLongitude{polyline_to_coordinate(longitude)},
-            util::FixedLatitude{static_cast<std::int32_t>(lat * detail::POLYLINE_TO_COORDINATE)};
+                             util::FixedLatitude{polyline_to_coordinate(latitude)}});
-        p.lon =
-            util::FixedLongitude{static_cast<std::int32_t>(lng * detail::POLYLINE_TO_COORDINATE)};
-        new_coordinates.push_back(p);
     }
 
-    return new_coordinates;
+    return coordinates;
 }
 }
 }

unit_tests/engine/geometry_string.cpp

@@ -33,7 +33,7 @@ BOOST_AUTO_TEST_CASE(decode)
 
     BOOST_CHECK_EQUAL(cmp_coords.size(), coords.size());
 
-    for (unsigned i = 0; i < cmp_coords.size(); ++i)
+    for (std::size_t i = 0; i < cmp_coords.size(); ++i)
     {
         BOOST_CHECK_CLOSE(static_cast<double>(util::toFloating(coords[i].lat)),
                           static_cast<double>(util::toFloating(cmp_coords[i].lat)),
@@ -86,4 +86,61 @@ BOOST_AUTO_TEST_CASE(encode6)
     BOOST_CHECK_EQUAL(encodedPolyline, polyline);
 }
 
+BOOST_AUTO_TEST_CASE(polyline_sign_check)
+{
+    // check sign conversion correctness from zig-zag encoding to two's complement
+    std::vector<util::Coordinate> coords = {
+        {util::FloatLongitude{0}, util::FloatLatitude{0}},
+        {util::FloatLongitude{-0.00001}, util::FloatLatitude{0.00000}},
+        {util::FloatLongitude{0.00000}, util::FloatLatitude{-0.00001}}};
+
+    const auto polyline = encodePolyline<100000>(coords.begin(), coords.end());
+    const auto result = decodePolyline(polyline);
+
+    BOOST_CHECK(coords.size() == result.size());
+    for (std::size_t i = 0; i < result.size(); ++i)
+    {
+        BOOST_CHECK(coords[i] == result[i]);
+    }
+}
+
+BOOST_AUTO_TEST_CASE(polyline_short_strings)
+{
+    // check zero longitude difference in the last coordinate
+    // the polyline is incorrect, but decodePolyline must not fail
+    std::vector<util::Coordinate> coords = {
+        {util::FloatLongitude{13.32476}, util::FloatLatitude{52.52632}},
+        {util::FloatLongitude{13.30179}, util::FloatLatitude{52.59155}},
+        {util::FloatLongitude{13.30179}, util::FloatLatitude{52.60391}}};
+
+    const auto polyline = encodePolyline<100000>(coords.begin(), coords.end());
+    BOOST_CHECK(polyline.back() == '?');
+
+    const auto result_short = decodePolyline(polyline.substr(0, polyline.size() - 1));
+    BOOST_CHECK(coords.size() == result_short.size());
+    for (std::size_t i = 0; i < result_short.size(); ++i)
+    {
+        BOOST_CHECK(coords[i] == result_short[i]);
+    }
+}
+
+BOOST_AUTO_TEST_CASE(incorrect_polylines)
+{
+    // check incorrect polylines
+    std::vector<std::string> polylines = {
+        "?",       // latitude only
+        "_",       // unfinished latitude
+        "?_",      // unfinished longitude
+        "?_______" // too long longitude (35 bits)
+    };
+    util::Coordinate coord{util::FloatLongitude{0}, util::FloatLatitude{0}};
+
+    for (auto polyline : polylines)
+    {
+        const auto result = decodePolyline(polyline);
+        BOOST_CHECK(result.size() == 1);
+        BOOST_CHECK(result.front() == coord);
+    }
+}
+
 BOOST_AUTO_TEST_SUITE_END()