Applied feedback from review

Author: glynos

include/network/uri/detail/encode.hpp

@@ -105,17 +105,6 @@ OutputIterator encode_path(InputIterator first, InputIterator last,
   return out;
 }
 
-template <typename InputIterator, typename OutputIterator>
-OutputIterator encode_query(InputIterator first, InputIterator last,
-                            OutputIterator out) {
-  auto it = first;
-  while (it != last) {
-    detail::encode_char(*it, out, "/.@&%;");
-    ++it;
-  }
-  return out;
-}
-
 template <typename InputIterator, typename OutputIterator>
 OutputIterator encode_query_component(InputIterator first, InputIterator last,
                                       OutputIterator out) {
@@ -167,13 +156,6 @@ String encode_path(const String &path) {
   return encoded;
 }
 
-template <class String>
-String encode_query(const String &query) {
-  String encoded;
-  encode_query(std::begin(query), std::end(query), std::back_inserter(encoded));
-  return encoded;
-}
-
 template <class String>
 String encode_fragment(const String &fragment) {
   String encoded;

include/network/uri/uri.hpp

@@ -560,7 +560,7 @@ class uri {
   template <typename InputIter, typename OutputIter>
   static OutputIter encode_query(InputIter first, InputIter last,
                                  OutputIter out) {
-    return detail::encode_query(first, last, out);
+    return detail::encode_query_component(first, last, out);
   }
 
   /**

include/network/uri/uri_builder.hpp

@@ -198,13 +198,13 @@ class uri_builder {
   /**
    * \brief Adds a new query value to the uri_builder. The '='
    *        symbol is percent encoded.
-   * \param parameter The query parameter.
+   * \param name The query parameter.
    * \returns \c *this
    * \sa append_query_key_value_pair
    */
   template <typename Source>
-  uri_builder &append_query_parameter(const Source &parameter) {
-    append_query_parameter(detail::translate(parameter));
+  uri_builder &append_query_parameter(const Source &name) {
+    append_query_parameter(detail::translate(name));
     return *this;
   }
 
@@ -261,7 +261,7 @@ class uri_builder {
   void set_port(string_type port);
   void set_authority(string_type authority);
   void set_path(string_type path);
-  void append_query_parameter(string_type query);
+  void append_query_parameter(string_type name);
   void append_query_key_value_pair(string_type key, string_type value);
   void set_fragment(string_type fragment);
 

src/uri_builder.cpp

@@ -12,31 +12,31 @@
 namespace network {
 uri_builder::uri_builder(const network::uri &base_uri) {
   if (base_uri.has_scheme()) {
-    set_scheme(base_uri.scheme().to_string());
+    scheme_ = base_uri.scheme().to_string();
   }
 
   if (base_uri.has_user_info()) {
-    set_user_info(base_uri.user_info().to_string());
+    user_info_ = base_uri.user_info().to_string();
   }
 
   if (base_uri.has_host()) {
-    set_host(base_uri.host().to_string());
+    host_ = base_uri.host().to_string();
   }
 
   if (base_uri.has_port()) {
-    set_port(base_uri.port().to_string());
+    port_ = base_uri.port().to_string();
   }
 
   if (base_uri.has_path()) {
-    set_path(base_uri.path().to_string());
+    path_ = base_uri.path().to_string();
   }
 
   if (base_uri.has_query()) {
-    append_query_parameter(base_uri.query().to_string());
+    query_ = base_uri.query().to_string();
   }
 
   if (base_uri.has_fragment()) {
-    set_fragment(base_uri.fragment().to_string());
+    fragment_ = base_uri.fragment().to_string();
   }
 }
 
@@ -111,14 +111,14 @@ uri_builder &uri_builder::clear_path() {
   return *this;
 }
 
-void uri_builder::append_query_parameter(string_type query) {
+void uri_builder::append_query_parameter(string_type name) {
   if (!query_) {
     query_ = string_type();
   }
   else {
     query_->append("&");
   }
-  network::uri::encode_query(std::begin(query), std::end(query),
+  network::uri::encode_query(std::begin(name), std::end(name),
                              std::back_inserter(*query_));
 }
 

test/uri_builder_test.cpp

@@ -838,3 +838,18 @@ TEST(builder_test, append_query_key_value_pair_does_not_encode_qmark) {
   ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "?"));
   ASSERT_EQ(network::string_view("?"), ub.uri().query_begin()->second);
 }
+
+TEST(builder_test, build_from_uri_with_encoded_user_info) {
+  network::uri_builder ub(network::uri("http://%[email protected]"));
+  ASSERT_EQ(network::string_view("%40"), ub.uri().user_info());
+}
+
+TEST(builder_test, build_from_uri_with_encoded_query) {
+  network::uri_builder ub(network::uri("http://example.com?x=%40"));
+  ASSERT_EQ(network::string_view("x=%40"), ub.uri().query());
+}
+
+TEST(builder_test, build_from_uri_with_encoded_fragment) {
+  network::uri_builder ub(network::uri("http://example.com#%40"));
+  ASSERT_EQ(network::string_view("%40"), ub.uri().fragment());
+}

test/uri_encoding_test.cpp

@@ -54,7 +54,7 @@ TEST(uri_encoding_test, encode_query_iterator) {
   std::string instance;
   network::uri::encode_query(std::begin(unencoded), std::end(unencoded),
 			     std::back_inserter(instance));
-  ASSERT_EQ("%21%23%24&%27%28%29%2A%2B%2C/%3A;%3D%3F@%5B%5D", instance);
+  ASSERT_EQ("%21%23%24%26%27%28%29%2A%2B%2C/%3A%3B%3D?%40%5B%5D", instance);
 }
 
 TEST(uri_encoding_test, encode_fragment_iterator) {