Applied changes to percent encoding of query parameters in the uri_builder

Author: glynos

include/network/uri/detail/encode.hpp

@@ -110,7 +110,7 @@ OutputIterator encode_query(InputIterator first, InputIterator last,
                             OutputIterator out) {
   auto it = first;
   while (it != last) {
-    detail::encode_char(*it, out, "/.@&%;=");
+    detail::encode_char(*it, out, "/.@&%;");
     ++it;
   }
   return out;

include/network/uri/uri_builder.hpp

@@ -182,13 +182,29 @@ class uri_builder {
   uri_builder &clear_path();
 
   /**
+   * \deprecated Please use append_query_parameter
+   * \warning This function's behaviour has changed and percent encoding
+   *          of the '=' character is not ignored.
    * \brief Adds a new query to the uri_builder.
    * \param query The query.
    * \returns \c *this
+   * \sa append_query_parameter
    */
   template <typename Source>
   uri_builder &append_query(const Source &query) {
-    append_query(detail::translate(query));
+    return append_query_parameter(query);
+  }
+
+  /**
+   * \brief Adds a new query value to the uri_builder. The '='
+   *        symbol is percent encoded.
+   * \param parameter 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));
     return *this;
   }
 
@@ -245,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(string_type query);
+  void append_query_parameter(string_type query);
   void append_query_key_value_pair(string_type key, string_type value);
   void set_fragment(string_type fragment);
 

src/uri.cpp

@@ -275,9 +275,7 @@ uri::query_iterator::query_iterator(optional<detail::uri_part> query)
 
 uri::query_iterator::query_iterator(const query_iterator &other)
   : query_(other.query_)
-  , kvp_(other.kvp_) {
-
-}
+  , kvp_(other.kvp_) {}
 
 uri::query_iterator &uri::query_iterator::operator = (const query_iterator &other) {
   auto tmp(other);

src/uri_builder.cpp

@@ -32,7 +32,7 @@ uri_builder::uri_builder(const network::uri &base_uri) {
   }
 
   if (base_uri.has_query()) {
-    append_query(base_uri.query().to_string());
+    append_query_parameter(base_uri.query().to_string());
   }
 
   if (base_uri.has_fragment()) {
@@ -111,7 +111,7 @@ uri_builder &uri_builder::clear_path() {
   return *this;
 }
 
-void uri_builder::append_query(string_type query) {
+void uri_builder::append_query_parameter(string_type query) {
   if (!query_) {
     query_ = string_type();
   }

test/uri_builder_test.cpp

@@ -217,7 +217,7 @@ TEST(builder_test, full_uri_doesnt_throw) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_NO_THROW(builder.uri());
@@ -231,10 +231,10 @@ TEST(builder_test, full_uri) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
-  ASSERT_EQ("http://[email protected]:80/path?query#fragment", builder.uri().string());
+  ASSERT_EQ("http://[email protected]:80/path?query=value#fragment", builder.uri().string());
 }
 
 TEST(builder_test, full_uri_has_scheme) {
@@ -245,7 +245,7 @@ TEST(builder_test, full_uri_has_scheme) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_scheme());
@@ -259,7 +259,7 @@ TEST(builder_test, full_uri_scheme_value) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_EQ("http", builder.uri().scheme());
@@ -273,7 +273,7 @@ TEST(builder_test, full_uri_has_user_info) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_user_info());
@@ -287,7 +287,7 @@ TEST(builder_test, full_uri_user_info_value) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_EQ("user", builder.uri().user_info());
@@ -301,7 +301,7 @@ TEST(builder_test, full_uri_has_host) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_host());
@@ -315,7 +315,7 @@ TEST(builder_test, full_uri_host_value) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_EQ("www.example.com", builder.uri().host());
@@ -329,7 +329,7 @@ TEST(builder_test, full_uri_has_port) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_port());
@@ -343,7 +343,7 @@ TEST(builder_test, full_uri_has_path) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_path());
@@ -357,7 +357,7 @@ TEST(builder_test, full_uri_path_value) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_EQ("/path", builder.uri().path());
@@ -371,7 +371,7 @@ TEST(builder_test, full_uri_has_query) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_query());
@@ -385,10 +385,10 @@ TEST(builder_test, full_uri_query_value) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
-  ASSERT_EQ("query", builder.uri().query());
+  ASSERT_EQ("query=value", builder.uri().query());
 }
 
 TEST(builder_test, full_uri_has_fragment) {
@@ -399,7 +399,7 @@ TEST(builder_test, full_uri_has_fragment) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_TRUE(builder.uri().has_fragment());
@@ -413,7 +413,7 @@ TEST(builder_test, full_uri_fragment_value) {
     .host("www.example.com")
     .port("80")
     .path("/path")
-    .append_query("query")
+    .append_query_key_value_pair("query", "value")
     .fragment("fragment")
     ;
   ASSERT_EQ("fragment", builder.uri().fragment());
@@ -718,17 +718,6 @@ TEST(builder_test, path_should_be_prefixed_with_slash_2) {
 }
 
 TEST(builder_test, set_multiple_query_with_encoding) {
-  network::uri_builder builder;
-  builder
-    .scheme("http")
-    .host("example.com")
-    .append_query("q1=foo bar")
-    .append_query("q2=biz baz")
-    ;
-  ASSERT_EQ("http://example.com?q1=foo%20bar&q2=biz%20baz", builder.uri().string());
-}
-
-TEST(builder_test, set_multiple_query_with_encoding_2) {
   network::uri_builder builder;
   builder
     .scheme("http")
@@ -799,9 +788,22 @@ TEST(builder_test, construct_from_uri_bug_116) {
   ASSERT_FALSE(c.has_port()) << c.string();
 }
 
+TEST(builder_test, append_query_value) {
+  network::uri_builder ub(network::uri("http://example.com"));
+  ASSERT_NO_THROW(ub.append_query_parameter("q"));
+  ASSERT_EQ(network::string_view("q"), ub.uri().query_begin()->first);
+}
+
+TEST(builder_test, append_query_value_encodes_equal_sign) {
+  network::uri_builder ub(network::uri("http://example.com"));
+  ASSERT_NO_THROW(ub.append_query_parameter("="));
+  ASSERT_EQ(network::string_view("%3D"), ub.uri().query_begin()->first);
+}
+
 TEST(builder_test, append_query_key_value_pair_encodes_equals_sign) {
   network::uri_builder ub(network::uri("http://example.com"));
   ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "="));
+  ASSERT_EQ(network::string_view("q"), ub.uri().query_begin()->first);
   ASSERT_EQ(network::string_view("%3D"), ub.uri().query_begin()->second);
 }
 

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;=%3F@%5B%5D", instance);
+  ASSERT_EQ("%21%23%24&%27%28%29%2A%2B%2C/%3A;%3D%3F@%5B%5D", instance);
 }
 
 TEST(uri_encoding_test, encode_fragment_iterator) {

test/uri_resolve_test.cpp

@@ -41,8 +41,8 @@ TEST_F(uri_resolve_test, base_has_empty_path__path_is_ref_path_1)
 
 TEST_F(uri_resolve_test, base_has_empty_path__path_is_ref_path_2)
 {
-  uri reference = uri_builder().path("g/x/y").append_query("q").fragment("s").uri();
-  ASSERT_EQ("http://a/g/x/y?q#s", resolved(uri("http://a/"), reference));
+  uri reference = uri_builder().path("g/x/y").append_query_key_value_pair("q", "1").fragment("s").uri();
+  ASSERT_EQ("http://a/g/x/y?q=1#s", resolved(uri("http://a/"), reference));
 }
 
 // normal examples
@@ -69,8 +69,8 @@ TEST_F(uri_resolve_test, path_starts_with_slash__path_is_ref_path) {
 }
 
 TEST_F(uri_resolve_test, path_starts_with_slash_with_query_fragment__path_is_ref_path) {
-  uri reference = uri_builder().path("/g/x").append_query("y").fragment("s").uri();
-  ASSERT_EQ("http://a/g/x?y#s", resolved(reference));
+  uri reference = uri_builder().path("/g/x").append_query_key_value_pair("y", "z").fragment("s").uri();
+  ASSERT_EQ("http://a/g/x?y=z#s", resolved(reference));
 }
 
 TEST_F(uri_resolve_test, DISABLED_has_authority__base_scheme_with_ref_authority) {
@@ -81,18 +81,18 @@ TEST_F(uri_resolve_test, DISABLED_has_authority__base_scheme_with_ref_authority)
 }
 
 TEST_F(uri_resolve_test, path_is_empty_but_has_query__returns_base_with_ref_query) {
-  uri reference = uri_builder().append_query("y").uri();
-  ASSERT_EQ("http://a/b/c/d;p?y", resolved(reference));
+  uri reference = uri_builder().append_query_key_value_pair("y", "z").uri();
+  ASSERT_EQ("http://a/b/c/d;p?y=z", resolved(reference));
 }
 
 TEST_F(uri_resolve_test, path_is_empty_but_has_query_base_no_query__returns_base_with_ref_query) {
-  uri reference = uri_builder().append_query("y").uri();
-  ASSERT_EQ("http://a/b/c/d?y", resolved(uri("http://a/b/c/d"), reference));
+  uri reference = uri_builder().append_query_key_value_pair("y", "z").uri();
+  ASSERT_EQ("http://a/b/c/d?y=z", resolved(uri("http://a/b/c/d"), reference));
 }
 
 TEST_F(uri_resolve_test, merge_path_with_query) {
-  uri reference = uri_builder().path("g").append_query("y").uri();
-  ASSERT_EQ("http://a/b/c/g?y", resolved(reference));
+  uri reference = uri_builder().path("g").append_query_key_value_pair("y", "z").uri();
+  ASSERT_EQ("http://a/b/c/g?y=z", resolved(reference));
 }
 
 TEST_F(uri_resolve_test, append_fragment) {
@@ -106,8 +106,8 @@ TEST_F(uri_resolve_test, merge_paths_with_fragment) {
 }
 
 TEST_F(uri_resolve_test, merge_paths_with_query_and_fragment) {
-  uri reference = uri_builder().path("g").append_query("y").fragment("s").uri();
-  ASSERT_EQ("http://a/b/c/g?y#s", resolved(reference));
+  uri reference = uri_builder().path("g").append_query_key_value_pair("y", "z").fragment("s").uri();
+  ASSERT_EQ("http://a/b/c/g?y=z#s", resolved(reference));
 }
 
 TEST_F(uri_resolve_test, merge_paths_with_semicolon_1) {
@@ -121,8 +121,8 @@ TEST_F(uri_resolve_test, merge_paths_with_semicolon_2) {
 }
 
 TEST_F(uri_resolve_test, merge_paths_with_semicolon_3) {
-  uri reference = uri_builder().path("g;x").append_query("y").fragment("s").uri();
-  ASSERT_EQ("http://a/b/c/g;x?y#s", resolved(reference));
+  uri reference = uri_builder().path("g;x").append_query_key_value_pair("y", "z").fragment("s").uri();
+  ASSERT_EQ("http://a/b/c/g;x?y=z#s", resolved(reference));
 }
 
 TEST_F(uri_resolve_test, path_is_empty__returns_base) {
@@ -245,12 +245,12 @@ TEST_F(uri_resolve_test, abnormal_example_14) {
 }
 
 TEST_F(uri_resolve_test, abnormal_example_15) {
-  uri reference = uri_builder().path("g").append_query("y/./x").uri();
+  uri reference = uri_builder().path("g").append_query_parameter("y/./x").uri();
   ASSERT_EQ("http://a/b/c/g?y/./x", resolved(reference));
 }
 
 TEST_F(uri_resolve_test, abnormal_example_16) {
-  uri reference = uri_builder().path("g").append_query("y/../x").uri();
+  uri reference = uri_builder().path("g").append_query_parameter("y/../x").uri();
   ASSERT_EQ("http://a/b/c/g?y/../x", resolved(reference));
 }