Skip to content

Applied changes to percent encoding of query parameters in the uri_builder #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 24, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/network/uri/detail/encode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 18 additions & 2 deletions include/network/uri/uri_builder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down
4 changes: 1 addition & 3 deletions src/uri.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/uri_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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();
}
Expand Down
58 changes: 30 additions & 28 deletions test/uri_builder_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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) {
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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) {
Expand All @@ -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());
Expand All @@ -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());
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion test/uri_encoding_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
32 changes: 16 additions & 16 deletions test/uri_resolve_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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));
}

Expand Down