Skip to content

Commit 659bc28

Browse files
glynosglynos
committed
Fixed how the query iterator increments and compares for equality.
1 parent 04f009f commit 659bc28

File tree

4 files changed

+81
-51
lines changed

4 files changed

+81
-51
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) Glyn Matthews 2012-2016.
1+
# Copyright (c) Glyn Matthews 2012-2017.
22
# Distributed under the Boost Software License, Version 1.0.
33
# (See accompanying file LICENSE_1_0.txt or copy at
44
# http://www.boost.org/LICENSE_1_0.txt)

‎include/network/uri/uri.hpp‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Copyright 2009-2010 Jeroen Habraken.
2-
// Copyright 2009-2016 Dean Michael Berris, Glyn Matthews.
2+
// Copyright 2009-2017 Dean Michael Berris, Glyn Matthews.
33
// Copyright 2012 Google, Inc.
44
// Distributed under the Boost Software License, Version 1.0.
55
// (See accompanying file LICENSE_1_0.txt or copy at
@@ -29,8 +29,8 @@
2929
#include <network/uri/detail/translate.hpp>
3030

3131
#ifdef NETWORK_URI_MSVC
32-
# pragma warning(push)
33-
# pragma warning(disable : 4251 4231 4660)
32+
#pragma warning(push)
33+
#pragma warning(disable : 4251 4231 4660)
3434
#endif
3535

3636
namespace network {
@@ -121,9 +121,8 @@ class uri {
121121
/**
122122
*
123123
*/
124-
class query_iterator
125-
{
126-
public:
124+
class query_iterator {
125+
public:
127126
using value_type = std::pair<string_view, string_view>;
128127
using difference_type = std::ptrdiff_t;
129128
using pointer = const value_type *;
@@ -133,25 +132,23 @@ class uri {
133132
query_iterator();
134133
explicit query_iterator(optional<detail::uri_part>);
135134
query_iterator(const query_iterator &);
136-
query_iterator &operator = (const query_iterator &);
137-
reference operator ++ () noexcept;
138-
value_type operator ++ (int) noexcept;
139-
reference operator * () const noexcept;
140-
pointer operator -> () const noexcept;
141-
bool operator == (const query_iterator &) const noexcept;
142-
inline bool operator != (const query_iterator &other) const noexcept {
135+
query_iterator &operator=(const query_iterator &);
136+
reference operator++() noexcept;
137+
value_type operator++(int) noexcept;
138+
reference operator*() const noexcept;
139+
pointer operator->() const noexcept;
140+
bool operator==(const query_iterator &) const noexcept;
141+
inline bool operator!=(const query_iterator &other) const noexcept {
143142
return !(*this == other);
144143
}
145144

146-
private:
147-
145+
private:
148146
void swap(query_iterator &) noexcept;
149-
void reset();
147+
void assign_kvp() noexcept;
150148
void increment() noexcept;
151149

152150
optional<detail::uri_part> query_;
153151
value_type kvp_;
154-
155152
};
156153

157154
/**
@@ -311,8 +308,8 @@ class uri {
311308
intT port(typename std::is_integral<intT>::type * = 0) const {
312309
assert(has_port());
313310
auto p = port();
314-
const char* port_first = std::addressof(*p.begin());
315-
char* port_last = 0;
311+
const char *port_first = std::addressof(*p.begin());
312+
char *port_last = 0;
316313
return static_cast<intT>(std::strtoul(port_first, &port_last, 10));
317314
}
318315

‎src/uri.cpp‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2012-2016 Glyn Matthews.
1+
// Copyright 2012-2017 Glyn Matthews.
22
// Copyright 2012 Google, Inc.
33
// Distributed under the Boost Software License, Version 1.0.
44
// (See accompanying file LICENSE_1_0.txt or copy at
@@ -259,16 +259,17 @@ uri::string_view uri::query() const noexcept {
259259
return has_query() ? to_string_view(uri_, *uri_parts_.query) : string_view{};
260260
}
261261

262-
uri::query_iterator::query_iterator()
263-
: query_{}
264-
, kvp_{} {
265-
reset();
266-
}
262+
uri::query_iterator::query_iterator() : query_{}, kvp_{} {}
267263

268264
uri::query_iterator::query_iterator(optional<detail::uri_part> query)
269265
: query_(query)
270266
, kvp_{} {
271-
reset();
267+
if (query_ && query_->empty()) {
268+
query_ = nullopt;
269+
}
270+
else {
271+
assign_kvp();
272+
}
272273
}
273274

274275
uri::query_iterator::query_iterator(const query_iterator &other)
@@ -302,43 +303,61 @@ uri::query_iterator::pointer uri::query_iterator::operator -> () const noexcept
302303
return std::addressof(kvp_);
303304
}
304305

305-
bool uri::query_iterator::operator == (const query_iterator &other) const noexcept {
306-
return kvp_.first == kvp_.second;
306+
bool uri::query_iterator::operator==(const query_iterator &other) const noexcept {
307+
if (!query_ && !other.query_) {
308+
return true;
309+
}
310+
else if (query_ && other.query_) {
311+
// since we're comparing substrings, the address of the first
312+
// element in each iterator must be the same
313+
return std::addressof(kvp_.first) == std::addressof(other.kvp_.first);
314+
}
315+
return false;
307316
}
308317

309318
void uri::query_iterator::swap(query_iterator &other) noexcept {
310319
std::swap(query_, other.query_);
311320
std::swap(kvp_, other.kvp_);
312321
}
313322

314-
void uri::query_iterator::reset() {
315-
kvp_ = value_type{};
316-
increment();
323+
void uri::query_iterator::assign_kvp() noexcept {
324+
auto first = std::begin(*query_), last = std::end(*query_);
325+
326+
auto sep_it =
327+
std::find_if(first, last,
328+
[](char c) -> bool { return c == '&' || c == ';'; });
329+
auto eq_it = std::find_if(first, sep_it,
330+
[](char c) -> bool { return c == '='; });
331+
332+
kvp_.first = string_view(std::addressof(*first), eq_it - first);
333+
if (eq_it != sep_it) {
334+
++eq_it; // skip '=' symbol
335+
}
336+
kvp_.second = string_view(std::addressof(*eq_it), sep_it - eq_it);
317337
}
318338

319339
void uri::query_iterator::increment() noexcept {
320-
if (query_ && !query_->empty()) {
321-
auto query = query_->to_string_view();
322-
auto first = query.begin(), last = query.end();
340+
assert(query_);
341+
342+
if (!query_->empty()) {
343+
auto first = std::begin(*query_), last = std::end(*query_);
323344

324345
auto sep_it =
325-
std::find_if(std::begin(query), std::end(query),
346+
std::find_if(first, last,
326347
[](char c) -> bool { return c == '&' || c == ';'; });
327-
auto eq_it = std::find_if(std::begin(query), sep_it,
328-
[](char c) -> bool { return c == '='; });
329-
330-
kvp_.first = string_view(std::addressof(*first), eq_it - first);
331-
if (eq_it != sep_it) {
332-
++eq_it; // skip '=' symbol
333-
}
334-
kvp_.second = string_view(std::addressof(*eq_it), sep_it - eq_it);
335348

336349
if (sep_it != last) {
337350
++sep_it; // skip next separator
338351
}
339352

340353
// reassign query to the next element
341354
query_ = detail::uri_part(sep_it, last);
355+
356+
assign_kvp();
357+
}
358+
359+
if (query_->empty()) {
360+
query_ = nullopt;
342361
}
343362
}
344363

@@ -562,7 +581,6 @@ uri uri::resolve(const uri &base) const {
562581
}
563582

564583
optional<uri::string_type> user_info, host, port, path, query, fragment;
565-
// const uri &base = *this;
566584

567585
if (has_authority()) {
568586
// g -> http://g

‎test/uri_test.cpp‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,7 @@ TEST(uri_test, non_opaque_path_has_double_slash) {
847847
TEST(uri_test, query_iterator_with_no_query) {
848848
network::uri instance("http://example.com/");
849849
ASSERT_FALSE(instance.has_query());
850+
ASSERT_EQ(instance.query_begin(), instance.query_end());
850851
}
851852

852853
TEST(uri_test, query_iterator_with_empty_query) {
@@ -858,34 +859,44 @@ TEST(uri_test, query_iterator_with_empty_query) {
858859
TEST(uri_test, query_iterator_with_single_kvp) {
859860
network::uri instance("http://example.com/?a=b");
860861
ASSERT_TRUE(instance.has_query());
861-
ASSERT_NE(instance.query_begin(), instance.query_end());
862-
auto kvp = *instance.query_begin();
863-
EXPECT_EQ("a", kvp.first);
864-
EXPECT_EQ("b", kvp.second);
862+
auto query_it = instance.query_begin();
863+
ASSERT_NE(query_it, instance.query_end());
864+
EXPECT_EQ("a", query_it->first);
865+
EXPECT_EQ("b", query_it->second);
866+
++query_it;
867+
EXPECT_EQ(query_it, instance.query_end());
865868
}
866869

867870
TEST(uri_test, query_iterator_with_two_kvps) {
868871
network::uri instance("http://example.com/?a=b&c=d");
872+
869873
ASSERT_TRUE(instance.has_query());
870-
ASSERT_NE(instance.query_begin(), instance.query_end());
871874
auto query_it = instance.query_begin();
875+
ASSERT_NE(query_it, instance.query_end());
872876
EXPECT_EQ("a", query_it->first);
873877
EXPECT_EQ("b", query_it->second);
874878
++query_it;
879+
ASSERT_NE(query_it, instance.query_end());
875880
EXPECT_EQ("c", query_it->first);
876881
EXPECT_EQ("d", query_it->second);
882+
++query_it;
883+
EXPECT_EQ(query_it, instance.query_end());
877884
}
878885

879886
TEST(uri_test, query_iterator_with_two_kvps_using_semicolon_separator) {
880887
network::uri instance("http://example.com/?a=b;c=d");
888+
881889
ASSERT_TRUE(instance.has_query());
882-
ASSERT_NE(instance.query_begin(), instance.query_end());
883890
auto query_it = instance.query_begin();
891+
ASSERT_NE(query_it, instance.query_end());
884892
EXPECT_EQ("a", query_it->first);
885893
EXPECT_EQ("b", query_it->second);
886894
++query_it;
895+
ASSERT_NE(query_it, instance.query_end());
887896
EXPECT_EQ("c", query_it->first);
888897
EXPECT_EQ("d", query_it->second);
898+
++query_it;
899+
EXPECT_EQ(query_it, instance.query_end());
889900
}
890901

891902
TEST(uri_test, query_iterator_with_key_and_no_value) {
@@ -894,6 +905,8 @@ TEST(uri_test, query_iterator_with_key_and_no_value) {
894905
auto query_it = instance.query_begin();
895906
EXPECT_EQ("query", query_it->first);
896907
EXPECT_EQ("", query_it->second);
908+
++query_it;
909+
EXPECT_EQ(query_it, instance.query_end());
897910
}
898911

899912
TEST(uri_test, query_iterator_with_fragment) {
@@ -906,6 +919,8 @@ TEST(uri_test, query_iterator_with_fragment) {
906919
++query_it;
907920
EXPECT_EQ("c", query_it->first);
908921
EXPECT_EQ("d", query_it->second);
922+
++query_it;
923+
EXPECT_EQ(query_it, instance.query_end());
909924
}
910925

911926
TEST(uri_test, copy_assignment_bug_98) {

0 commit comments

Comments
 (0)