Skip to content

Commit ee74d8a

Browse files
mtrenkmannglynos
mtrenkmann
authored and
glynos
committed
Fix percent encoding in query component (#120)
* Add functions is_unreserved and is_sub_delim * Make use of is_unreserved() function * Add and use percent_encode() function * Add encode_pchar() function * Add ignore parameter to encode_pchar() function * Make encode_query() make use of encode_pchar() function * Fix typo * Adopt unit test to accepts fixed query encoding * Fix invalid escape sequence * Adopt unit test to accepts fixed query encoding * Remove obsolete comment * Make encode_query() use encode_char() again * Add tests to check encoding of delimiters when used as query parameter values * Add encode_query_component() function * Change append_query_key_value_pair() to encode components separately * Roll back some previous changes * Remove obsolete code * Do not encode slash and qmark in query component encoding * Roll back
1 parent 0bfbd14 commit ee74d8a

File tree

5 files changed

+95
-29
lines changed

5 files changed

+95
-29
lines changed

‎include/network/uri/detail/encode.hpp‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,24 +29,34 @@ inline CharT hex_to_letter(CharT in) {
2929
return in;
3030
}
3131

32+
template <class charT, class OutputIterator>
33+
void percent_encode(charT in, OutputIterator &out) {
34+
out++ = '%';
35+
out++ = hex_to_letter((in >> 4) & 0x0f);
36+
out++ = hex_to_letter(in & 0x0f);
37+
}
38+
39+
template <class charT>
40+
bool is_unreserved(charT in) {
41+
return ((in >= 'a') && (in <= 'z')) ||
42+
((in >= 'A') && (in <= 'Z')) ||
43+
((in >= '0') && (in <= '9')) ||
44+
(in == '-') ||
45+
(in == '.') ||
46+
(in == '_') ||
47+
(in == '~');
48+
}
49+
3250
template <class charT, class OutputIterator>
3351
void encode_char(charT in, OutputIterator &out, const char *ignore = "") {
34-
if (((in >= 'a') && (in <= 'z')) ||
35-
((in >= 'A') && (in <= 'Z')) ||
36-
((in >= '0') && (in <= '9')) ||
37-
(in == '-') ||
38-
(in == '.') ||
39-
(in == '_') ||
40-
(in == '~')) {
52+
if (is_unreserved(in)) {
4153
out++ = in;
4254
} else {
4355
auto first = ignore, last = ignore + std::strlen(ignore);
4456
if (std::find(first, last, in) != last) {
4557
out++ = in;
4658
} else {
47-
out++ = '%';
48-
out++ = hex_to_letter((in >> 4) & 0x0f);
49-
out++ = hex_to_letter(in & 0x0f);
59+
percent_encode(in, out);
5060
}
5161
}
5262
}
@@ -106,6 +116,17 @@ OutputIterator encode_query(InputIterator first, InputIterator last,
106116
return out;
107117
}
108118

119+
template <typename InputIterator, typename OutputIterator>
120+
OutputIterator encode_query_component(InputIterator first, InputIterator last,
121+
OutputIterator out) {
122+
auto it = first;
123+
while (it != last) {
124+
detail::encode_char(*it, out, "/?");
125+
++it;
126+
}
127+
return out;
128+
}
129+
109130
template <typename InputIterator, typename OutputIterator>
110131
OutputIterator encode_fragment(InputIterator first, InputIterator last,
111132
OutputIterator out) {

‎include/network/uri/uri_builder.hpp‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -199,22 +199,15 @@ class uri_builder {
199199
uri_builder &clear_query();
200200

201201
/**
202-
* \brief Adds a new query to the uri_builder.
202+
* \brief Adds a new query key/value pair to the uri_builder.
203203
* \param key The query key.
204204
* \param value The query value.
205205
* \returns \c *this
206206
*/
207207
template <typename Key, typename Value>
208-
uri_builder &append_query_key_value_pair(const Key &key, const Value &value) {
209-
if (!query_) {
210-
query_ = string_type();
211-
}
212-
else {
213-
query_->append("&");
214-
}
215-
string_type query_pair = detail::translate(key) + "=" + detail::translate(value);
216-
network::uri::encode_query(std::begin(query_pair), std::end(query_pair),
217-
std::back_inserter(*query_));
208+
uri_builder &append_query_key_value_pair(const Key &key, const Value &value) {
209+
append_query_key_value_pair(detail::translate(key),
210+
detail::translate(value));
218211
return *this;
219212
}
220213

@@ -253,6 +246,7 @@ class uri_builder {
253246
void set_authority(string_type authority);
254247
void set_path(string_type path);
255248
void append_query(string_type query);
249+
void append_query_key_value_pair(string_type key, string_type value);
256250
void set_fragment(string_type fragment);
257251

258252
optional<string_type> scheme_, user_info_, host_, port_, path_, query_,

‎src/uri_builder.cpp‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,19 @@ void uri_builder::append_query(string_type query) {
122122
std::back_inserter(*query_));
123123
}
124124

125+
void uri_builder::append_query_key_value_pair(string_type key, string_type value) {
126+
if (!query_) {
127+
query_ = string_type();
128+
} else {
129+
query_->push_back('&');
130+
}
131+
detail::encode_query_component(std::begin(key), std::end(key),
132+
std::back_inserter(*query_));
133+
query_->push_back('=');
134+
detail::encode_query_component(std::begin(value), std::end(value),
135+
std::back_inserter(*query_));
136+
}
137+
125138
uri_builder &uri_builder::clear_query() {
126139
query_ = network::nullopt;
127140
return *this;

‎test/uri_builder_test.cpp‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -794,7 +794,45 @@ TEST(builder_test, construct_from_uri_bug_116) {
794794
const network::uri b("http://b.com");
795795
a = b;
796796

797-
network::uri_builder ub(a); // ASAN reports heap-use-after-free here
797+
network::uri_builder ub(a);
798798
const network::uri c(ub.uri());
799799
ASSERT_FALSE(c.has_port()) << c.string();
800800
}
801+
802+
TEST(builder_test, append_query_key_value_pair_encodes_equals_sign) {
803+
network::uri_builder ub(network::uri("http://example.com"));
804+
ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "="));
805+
ASSERT_EQ(network::string_view("%3D"), ub.uri().query_begin()->second);
806+
}
807+
808+
TEST(builder_test, append_query_key_value_pair_encodes_number_sign) {
809+
network::uri_builder ub(network::uri("http://example.com"));
810+
ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "#"));
811+
ASSERT_EQ(network::string_view("%23"), ub.uri().query_begin()->second);
812+
}
813+
814+
TEST(builder_test, append_query_key_value_pair_encodes_percent_sign) {
815+
network::uri_builder ub(network::uri("http://example.com"));
816+
ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "%"));
817+
ASSERT_EQ(network::string_view("%25"), ub.uri().query_begin()->second);
818+
}
819+
820+
TEST(builder_test, append_query_key_value_pair_encodes_ampersand) {
821+
network::uri_builder ub(network::uri("http://example.com"));
822+
ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "&"));
823+
ASSERT_EQ(network::string_view("%26"), ub.uri().query_begin()->second);
824+
}
825+
826+
TEST(builder_test, append_query_key_value_pair_does_not_encode_slash) {
827+
// https://tools.ietf.org/html/rfc3986#section-3.4
828+
network::uri_builder ub(network::uri("http://example.com"));
829+
ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "/"));
830+
ASSERT_EQ(network::string_view("/"), ub.uri().query_begin()->second);
831+
}
832+
833+
TEST(builder_test, append_query_key_value_pair_does_not_encode_qmark) {
834+
// https://tools.ietf.org/html/rfc3986#section-3.4
835+
network::uri_builder ub(network::uri("http://example.com"));
836+
ASSERT_NO_THROW(ub.append_query_key_value_pair("q", "?"));
837+
ASSERT_EQ(network::string_view("?"), ub.uri().query_begin()->second);
838+
}

‎test/uri_encoding_test.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@
1010

1111

1212
TEST(uri_encoding_test, encode_user_info_iterator) {
13-
const std::string unencoded("!#$&\'()*+,/:;=?@[]");
13+
const std::string unencoded("!#$&'()*+,/:;=?@[]");
1414
std::string instance;
1515
network::uri::encode_user_info(std::begin(unencoded), std::end(unencoded),
1616
std::back_inserter(instance));
1717
ASSERT_EQ("%21%23%24%26%27%28%29%2A%2B%2C%2F:%3B%3D%3F%40%5B%5D", instance);
1818
}
1919

2020
TEST(uri_encoding_test, encode_host_iterator) {
21-
const std::string unencoded("!#$&\'()*+,/:;=?@[]");
21+
const std::string unencoded("!#$&'()*+,/:;=?@[]");
2222
std::string instance;
2323
network::uri::encode_host(std::begin(unencoded), std::end(unencoded),
2424
std::back_inserter(instance));
@@ -34,31 +34,31 @@ TEST(uri_encoding_test, encode_ipv6_host) {
3434
}
3535

3636
TEST(uri_encoding_test, encode_port_iterator) {
37-
const std::string unencoded("!#$&\'()*+,/:;=?@[]");
37+
const std::string unencoded("!#$&'()*+,/:;=?@[]");
3838
std::string instance;
3939
network::uri::encode_port(std::begin(unencoded), std::end(unencoded),
4040
std::back_inserter(instance));
4141
ASSERT_EQ("%21%23%24%26%27%28%29%2A%2B%2C%2F%3A%3B%3D%3F%40%5B%5D", instance);
4242
}
4343

4444
TEST(uri_encoding_test, encode_path_iterator) {
45-
const std::string unencoded("!#$&\'()*+,/:;=?@[]");
45+
const std::string unencoded("!#$&'()*+,/:;=?@[]");
4646
std::string instance;
4747
network::uri::encode_path(std::begin(unencoded), std::end(unencoded),
4848
std::back_inserter(instance));
4949
ASSERT_EQ("%21%23%24%26%27%28%29%2A%2B%2C/%3A;=%3F@%5B%5D", instance);
5050
}
5151

5252
TEST(uri_encoding_test, encode_query_iterator) {
53-
const std::string unencoded("!#$&\'()*+,/:;=?@[]");
53+
const std::string unencoded("!#$&'()*+,/:;=?@[]");
5454
std::string instance;
5555
network::uri::encode_query(std::begin(unencoded), std::end(unencoded),
5656
std::back_inserter(instance));
5757
ASSERT_EQ("%21%23%24&%27%28%29%2A%2B%2C/%3A;=%3F@%5B%5D", instance);
5858
}
5959

6060
TEST(uri_encoding_test, encode_fragment_iterator) {
61-
const std::string unencoded("!#$&\'()*+,/:;=?@[]");
61+
const std::string unencoded("!#$&'()*+,/:;=?@[]");
6262
std::string instance;
6363
network::uri::encode_fragment(std::begin(unencoded), std::end(unencoded),
6464
std::back_inserter(instance));
@@ -70,7 +70,7 @@ TEST(uri_encoding_test, decode_iterator) {
7070
std::string instance;
7171
network::uri::decode(std::begin(encoded), std::end(encoded),
7272
std::back_inserter(instance));
73-
ASSERT_EQ("!#$&\'()*+,/:;=?@[]", instance);
73+
ASSERT_EQ("!#$&'()*+,/:;=?@[]", instance);
7474
}
7575

7676
TEST(uri_encoding_test, decode_iterator_error_1) {

0 commit comments

Comments
 (0)