Merge pull request #1397 from Project-OSRM/escape-fix

JSON string escape fix

algorithms/polyline_compressor.cpp

@@ -56,19 +56,11 @@ std::string PolylineCompressor::encode_number(int number_to_encode) const
     {
         const int next_value = (0x20 | (number_to_encode & 0x1f)) + 63;
         output += static_cast<char>(next_value);
-        if (92 == next_value)
-        {
-            output += static_cast<char>(next_value);
-        }
         number_to_encode >>= 5;
     }
 
     number_to_encode += 63;
     output += static_cast<char>(number_to_encode);
-    if (92 == number_to_encode)
-    {
-        output += static_cast<char>(number_to_encode);
-    }
     return output;
 }
 

algorithms/route_name_extraction.hpp

@@ -1,6 +1,6 @@
 /*
 
-Copyright (c) 2013, Project OSRM contributors
+Copyright (c) 2015, Project OSRM contributors
 All rights reserved.
 
 Redistribution and use in source and binary forms, with or without modification,
@@ -147,15 +147,13 @@ template <class DataFacadeT, class SegmentT> struct ExtractRouteNames
         }
 
         // fetching names for the selected segments
-        route_names.shortest_path_name_1 =
+        route_names.shortest_path_name_1 = facade->get_name_for_id(shortest_segment_1.name_id);
-            facade->GetEscapedNameForNameID(shortest_segment_1.name_id);
+        route_names.shortest_path_name_2 = facade->get_name_for_id(shortest_segment_2.name_id);
-        route_names.shortest_path_name_2 =
-            facade->GetEscapedNameForNameID(shortest_segment_2.name_id);
 
         route_names.alternative_path_name_1 =
-            facade->GetEscapedNameForNameID(alternative_segment_1.name_id);
+            facade->get_name_for_id(alternative_segment_1.name_id);
         route_names.alternative_path_name_2 =
-            facade->GetEscapedNameForNameID(alternative_segment_2.name_id);
+            facade->get_name_for_id(alternative_segment_2.name_id);
 
         return route_names;
     }

descriptors/descriptor_base.hpp

@@ -33,6 +33,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "../data_structures/phantom_node.hpp"
 #include "../typedefs.h"
 
+#include <boost/assert.hpp>
+
 #include <osrm/json_container.hpp>
 
 #include <string>

descriptors/json_descriptor.hpp

@@ -113,9 +113,6 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
         }
 
         // check if first segment is non-zero
-        std::string road_name = facade->GetEscapedNameForNameID(
-            raw_route.segment_end_coordinates.front().source_phantom.name_id);
-
         BOOST_ASSERT(raw_route.unpacked_path_segments.size() ==
                      raw_route.segment_end_coordinates.size());
 
@@ -157,9 +154,9 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
         json_route_summary.values["total_distance"] = description_factory.summary.distance;
         json_route_summary.values["total_time"] = description_factory.summary.duration;
         json_route_summary.values["start_point"] =
-            facade->GetEscapedNameForNameID(description_factory.summary.source_name_id);
+            facade->get_name_for_id(description_factory.summary.source_name_id);
         json_route_summary.values["end_point"] =
-            facade->GetEscapedNameForNameID(description_factory.summary.target_name_id);
+            facade->get_name_for_id(description_factory.summary.target_name_id);
         json_result.values["route_summary"] = json_route_summary;
 
         BOOST_ASSERT(!raw_route.segment_end_coordinates.empty());
@@ -241,10 +238,10 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
                 alternate_description_factory.summary.distance;
             json_alternate_route_summary.values["total_time"] =
                 alternate_description_factory.summary.duration;
-            json_alternate_route_summary.values["start_point"] = facade->GetEscapedNameForNameID(
+            json_alternate_route_summary.values["start_point"] =
-                alternate_description_factory.summary.source_name_id);
+                facade->get_name_for_id(alternate_description_factory.summary.source_name_id);
-            json_alternate_route_summary.values["end_point"] = facade->GetEscapedNameForNameID(
+            json_alternate_route_summary.values["end_point"] =
-                alternate_description_factory.summary.target_name_id);
+                facade->get_name_for_id(alternate_description_factory.summary.target_name_id);
             json_alternate_route_summary_array.values.push_back(json_alternate_route_summary);
             json_result.values["alternative_summaries"] = json_alternate_route_summary_array;
 
@@ -349,8 +346,7 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
                     }
                     json_instruction_row.values.push_back(current_turn_instruction);
 
-                    json_instruction_row.values.push_back(
+                    json_instruction_row.values.push_back(facade->get_name_for_id(segment.name_id));
-                        facade->GetEscapedNameForNameID(segment.name_id));
                     json_instruction_row.values.push_back(std::round(segment.length));
                     json_instruction_row.values.push_back(necessary_segments_running_index);
                     json_instruction_row.values.push_back(std::round(segment.duration / 10.));

plugins/nearest.hpp

@@ -88,9 +88,8 @@ template <class DataFacadeT> class NearestPlugin final : public BasePlugin
                     json_coordinate.values.push_back(phantom_node_vector.at(i).location.lon /
                                                      COORDINATE_PRECISION);
                     result.values["mapped coordinate"] = json_coordinate;
-                    std::string temp_string;
+                    result.values["name"] =
-                    facade->GetName(phantom_node_vector.at(i).name_id, temp_string);
+                        facade->get_name_for_id(phantom_node_vector.at(i).name_id);
-                    result.values["name"] = temp_string;
                     results.values.push_back(result);
                 }
                 json_result.values["results"] = results;
@@ -103,9 +102,8 @@ template <class DataFacadeT> class NearestPlugin final : public BasePlugin
                 json_coordinate.values.push_back(phantom_node_vector.front().location.lon /
                                                  COORDINATE_PRECISION);
                 json_result.values["mapped_coordinate"] = json_coordinate;
-                std::string temp_string;
+                json_result.values["name"] =
-                facade->GetName(phantom_node_vector.front().name_id, temp_string);
+                    facade->get_name_for_id(phantom_node_vector.front().name_id);
-                json_result.values["name"] = temp_string;
             }
         }
         return 200;

server/data_structures/datafacade_base.hpp

@@ -109,14 +109,7 @@ template <class EdgeDataT> class BaseDataFacade
 
     virtual unsigned GetNameIndexFromEdgeID(const unsigned id) const = 0;
 
-    virtual void GetName(const unsigned name_id, std::string &result) const = 0;
+    virtual std::string get_name_for_id(const unsigned name_id) const = 0;
-
-    std::string GetEscapedNameForNameID(const unsigned name_id) const
-    {
-        std::string temporary_string;
-        GetName(name_id, temporary_string);
-        return EscapeJSONString(temporary_string);
-    }
 
     virtual std::string GetTimestamp() const = 0;
 };

server/data_structures/internal_datafacade.hpp

@@ -46,6 +46,8 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <osrm/coordinate.hpp>
 #include <osrm/server_paths.hpp>
 
+#include <limits>
+
 template <class EdgeDataT> class InternalDataFacade final : public BaseDataFacade<EdgeDataT>
 {
 
@@ -418,24 +420,25 @@ template <class EdgeDataT> class InternalDataFacade final : public BaseDataFacad
     unsigned GetNameIndexFromEdgeID(const unsigned id) const override final
     {
         return m_name_ID_list.at(id);
-    };
+    }
 
-    void GetName(const unsigned name_id, std::string &result) const override final
+    std::string get_name_for_id(const unsigned name_id) const override final
     {
-        if (UINT_MAX == name_id)
+        if (std::numeric_limits<unsigned>::max() == name_id)
         {
-            result = "";
+            return "";
-            return;
         }
         auto range = m_name_table.GetRange(name_id);
 
-        result.clear();
+        std::string result;
+        result.reserve(range.size());
         if (range.begin() != range.end())
         {
             result.resize(range.back() - range.front() + 1);
             std::copy(m_names_char_list.begin() + range.front(),
                       m_names_char_list.begin() + range.back() + 1, result.begin());
         }
+        return result;
     }
 
     virtual unsigned GetGeometryIndexForEdgeID(const unsigned id) const override final

server/data_structures/shared_datafacade.hpp

@@ -41,6 +41,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "../../util/simple_logger.hpp"
 
 #include <algorithm>
+#include <limits>
 #include <memory>
 
 template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<EdgeDataT>
@@ -409,22 +410,23 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
         return m_name_ID_list.at(id);
     };
 
-    void GetName(const unsigned name_id, std::string &result) const override final
+    std::string get_name_for_id(const unsigned name_id) const override final
     {
-        if (UINT_MAX == name_id)
+        if (std::numeric_limits<unsigned>::max() == name_id)
         {
-            result = "";
+            return "";
-            return;
         }
         auto range = m_name_table->GetRange(name_id);
 
-        result.clear();
+        std::string result;
+        result.reserve(range.size());
         if (range.begin() != range.end())
         {
             result.resize(range.back() - range.front() + 1);
             std::copy(m_names_char_list.begin() + range.front(),
                       m_names_char_list.begin() + range.back() + 1, result.begin());
         }
+        return result;
     }
 
     std::string GetTimestamp() const override final { return m_timestamp; }

util/integer_range.hpp

@@ -50,6 +50,7 @@ template <typename Integer> class range
     const range &end() const { return *this; }
     Integer front() const { return iter; }
     Integer back() const { return last - 1; }
+    Integer size() const { return last - iter; }
 
     // Iterator functions
     bool operator!=(const range &) const { return iter < last; }

util/json_renderer.hpp

@@ -32,6 +32,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define JSON_RENDERER_HPP
 
 #include "cast.hpp"
+#include "string_util.hpp"
 
 #include <osrm/json_container.hpp>
 
@@ -44,7 +45,12 @@ struct Renderer : mapbox::util::static_visitor<>
 {
     explicit Renderer(std::ostream &_out) : out(_out) {}
 
-    void operator()(const String &string) const { out << "\"" << string.value << "\""; }
+    void operator()(const String &string) const
+    {
+        out << "\"";
+        out << escape_JSON(string.value);
+        out << "\"";
+    }
 
     void operator()(const Number &number) const
     {
@@ -101,7 +107,8 @@ struct ArrayRenderer : mapbox::util::static_visitor<>
     void operator()(const String &string) const
     {
         out.push_back('\"');
-        out.insert(out.end(), string.value.begin(), string.value.end());
+        const auto string_to_insert = escape_JSON(string.value);
+        out.insert(std::end(out), std::begin(string_to_insert), std::end(string_to_insert));
         out.push_back('\"');
     }
 

util/string_util.hpp

@@ -77,10 +77,17 @@ inline void replaceAll(std::string &s, const std::string &sub, const std::string
     boost::replace_all(s, sub, other);
 }
 
-inline std::string EscapeJSONString(const std::string &input)
+inline std::string escape_JSON(const std::string &input)
 {
+    // return the input if no backslash can be found -> no allocations
+    if (input.find_first_of('\\') == std::string::npos)
+    {
+        return input;
+    }
+
+    // escape and skip reallocations if possible
     std::string output;
-    output.reserve(input.size());
+    output.reserve(input.size() + 4); // +4 assumes two backslashes on avg
     for (auto iter = input.begin(); iter != input.end(); ++iter)
     {
         switch (iter[0])