Skip to content

Commit b91cacc

Browse files
glynosglynos
authored
Merge pull request #99 from glynos/copy_assignment_bug_issue_98
Copy assignment bug issue 98
2 parents c515c75 + 804b330 commit b91cacc

File tree

7 files changed

+180
-23
lines changed

7 files changed

+180
-23
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,17 @@ class uri_part {
5252
};
5353

5454
struct hierarchical_part {
55+
hierarchical_part() = default;
56+
5557
optional<uri_part> user_info;
5658
optional<uri_part> host;
5759
optional<uri_part> port;
5860
optional<uri_part> path;
5961
};
6062

6163
struct uri_parts {
64+
uri_parts() = default;
65+
6266
optional<uri_part> scheme;
6367
hierarchical_part hier_part;
6468
optional<uri_part> query;

‎src/detail/uri_advance_parts.cpp‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2016 Glyn Matthews.
1+
// Copyright 2016-2017 Glyn Matthews.
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)
@@ -24,7 +24,7 @@ uri_part copy_part(const std::string &uri, string_view::const_iterator &it) {
2424
return copy_part(std::begin(uri), std::end(uri), it);
2525
}
2626

27-
void advance_parts(string_view &uri_view, uri_parts &parts,
27+
void advance_parts(string_view uri_view, uri_parts &parts,
2828
const uri_parts &existing_parts) {
2929
auto first = std::begin(uri_view);
3030

@@ -39,9 +39,7 @@ void advance_parts(string_view &uri_view, uri_parts &parts,
3939

4040
// ignore // for hierarchical URIs
4141
if (existing_parts.hier_part.host) {
42-
while (*it == '/') {
43-
++it;
44-
}
42+
std::advance(it, 2);
4543
}
4644
}
4745

‎src/detail/uri_advance_parts.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace detail {
1313
uri_part copy_part(const std::string &part,
1414
string_view::const_iterator &it);
1515

16-
void advance_parts(string_view &uri_view, uri_parts &parts,
16+
void advance_parts(string_view uri_view, uri_parts &parts,
1717
const uri_parts &existing_parts);
1818
} // namespace detail
1919
} // namespace network

‎src/detail/uri_parse.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2016 Glyn Matthews.
1+
// Copyright 2016-2017 Glyn Matthews.
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)

‎src/uri.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ void uri::initialize(optional<string_type> scheme,
102102
}
103103

104104
if (path) {
105-
// if the URI is hierarchical and the path is not already
106-
// prefixed with a '/', add one.
105+
// if the URI is not opaque and the path is not already prefixed
106+
// with a '/', add one.
107107
if (host && (!path->empty() && path->front() != '/')) {
108108
path = "/" + *path;
109109
}
@@ -194,10 +194,11 @@ uri &uri::operator=(uri other) {
194194
}
195195

196196
void uri::swap(uri &other) noexcept {
197+
auto parts = uri_parts_;
197198
advance_parts(other.uri_view_, uri_parts_, other.uri_parts_);
198199
uri_.swap(other.uri_);
199200
uri_view_.swap(other.uri_view_);
200-
advance_parts(other.uri_view_, other.uri_parts_, uri_parts_);
201+
advance_parts(other.uri_view_, other.uri_parts_, parts);
201202
}
202203

203204
uri::const_iterator uri::begin() const noexcept { return uri_view_.begin(); }

‎test/uri_parse_test.cpp‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2016 Glyn Matthews.
1+
// Copyright 2016-2017 Glyn Matthews.
22
// Distributed under the Boost Software License, Version 1.0.
33
// (See accompanying file LICENSE_1_0.txt of copy at
44
// http://www.boost.org/LICENSE_1_0.txt)
@@ -485,14 +485,52 @@ TEST(uri_parse_test, test_pct_encoded_user_info) {
485485
EXPECT_EQ("/", uri.path());
486486
}
487487

488+
TEST(uri_parse_test, test_file_uri_bug_98) {
489+
test::uri uri("file:///bin/bash");
490+
EXPECT_TRUE(uri.parse_uri());
491+
ASSERT_FALSE(uri.has_user_info());
492+
ASSERT_TRUE(uri.has_host());
493+
EXPECT_EQ("", uri.host());
494+
ASSERT_TRUE(uri.has_path());
495+
EXPECT_EQ("/bin/bash", uri.path());
496+
}
497+
498+
TEST(uri_parse_test, test_file_uri_bug_98_2) {
499+
test::uri uri("file://localhost/bin");
500+
EXPECT_TRUE(uri.parse_uri());
501+
ASSERT_FALSE(uri.has_user_info());
502+
ASSERT_TRUE(uri.has_host());
503+
EXPECT_EQ("localhost", uri.host());
504+
ASSERT_TRUE(uri.has_path());
505+
EXPECT_EQ("/bin", uri.path());
506+
}
507+
508+
TEST(uri_parse_test, test_file_uri_bug_98_3) {
509+
test::uri uri("file://localhost/bin/bash");
510+
EXPECT_TRUE(uri.parse_uri());
511+
ASSERT_FALSE(uri.has_user_info());
512+
ASSERT_TRUE(uri.has_host());
513+
EXPECT_EQ("localhost", uri.host());
514+
ASSERT_TRUE(uri.has_path());
515+
EXPECT_EQ("/bin/bash", uri.path());
516+
}
517+
518+
TEST(uri_parse_test, test_file_uri_bug_98_4) {
519+
test::uri uri("file://localhost/bin");
520+
EXPECT_TRUE(uri.parse_uri());
521+
ASSERT_FALSE(uri.has_user_info());
522+
ASSERT_TRUE(uri.has_host());
523+
EXPECT_EQ("localhost", uri.host());
524+
ASSERT_TRUE(uri.has_path());
525+
EXPECT_EQ("/bin", uri.path());
526+
}
527+
488528
// http://formvalidation.io/validators/uri/
489529

490530
std::vector<std::string> create_urls(const std::string &filename) {
491531
std::vector<std::string> urls;
492532
std::ifstream ifs(filename);
493-
std::cout << filename << std::endl;
494533
if (!ifs) {
495-
std::cout << "Shit." << std::endl;
496534
throw std::runtime_error("Unable to open file: " + filename);
497535
}
498536
for (std::string url; std::getline(ifs, url);) {

‎test/uri_test.cpp‎

Lines changed: 126 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// Copyright 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 of copy at
@@ -223,6 +223,17 @@ TEST(uri_test, file_test) {
223223
EXPECT_EQ("/bin/bash", instance.path());
224224
}
225225

226+
TEST(uri_test, file_path_has_host_bug_98) {
227+
network::uri instance("file:///bin/bash");
228+
EXPECT_TRUE(instance.has_scheme());
229+
EXPECT_FALSE(instance.has_user_info());
230+
EXPECT_TRUE(instance.has_host());
231+
EXPECT_FALSE(instance.has_port());
232+
EXPECT_TRUE(instance.has_path());
233+
EXPECT_FALSE(instance.has_query());
234+
EXPECT_FALSE(instance.has_fragment());
235+
}
236+
226237
TEST(uri_test, xmpp_test) {
227238
network::uri instance("xmpp:example-node@example.com?message;subject=Hello%20World");
228239
EXPECT_EQ("xmpp", instance.scheme());
@@ -418,11 +429,23 @@ TEST(uri_test, assignment_test) {
418429
}
419430

420431
TEST(uri_test, swap_test) {
421-
network::uri instance("http://www.example.com/");
422-
network::uri copy("http://www.example.org/");
423-
network::swap(instance, copy);
424-
EXPECT_EQ("http://www.example.org/", instance);
425-
EXPECT_EQ("http://www.example.com/", copy);
432+
network::uri original("http://example.com/path/to/file.txt");
433+
network::uri instance("file:///something/different/");
434+
original.swap(instance);
435+
436+
ASSERT_TRUE(original.has_scheme());
437+
ASSERT_TRUE(original.has_host());
438+
ASSERT_TRUE(original.has_path());
439+
EXPECT_EQ("file", original.scheme());
440+
EXPECT_EQ("", original.host());
441+
EXPECT_EQ("/something/different", original.path());
442+
443+
ASSERT_TRUE(instance.has_scheme());
444+
ASSERT_TRUE(instance.has_host());
445+
ASSERT_TRUE(instance.has_path());
446+
EXPECT_EQ("http", instance.scheme());
447+
EXPECT_EQ("example.com", instance.host());
448+
EXPECT_EQ("/path/to/file.txt", instance.path());
426449
}
427450

428451
TEST(uri_test, authority_test) {
@@ -531,14 +554,14 @@ TEST(uri_test, mailto_has_no_authority) {
531554
EXPECT_FALSE(instance.has_authority());
532555
}
533556

534-
TEST(uri_test, http_is_hierarchical) {
557+
TEST(uri_test, http_is_not_opaque) {
535558
network::uri instance("http://www.example.com/");
536-
EXPECT_TRUE(!instance.is_opaque());
559+
EXPECT_FALSE(instance.is_opaque());
537560
}
538561

539-
TEST(uri_test, file_is_hierarchical) {
562+
TEST(uri_test, file_is_not_opaque) {
540563
network::uri instance("file:///bin/bash");
541-
EXPECT_TRUE(!instance.is_opaque());
564+
EXPECT_FALSE(instance.is_opaque());
542565
}
543566

544567
TEST(uri_test, mailto_is_absolute) {
@@ -884,3 +907,96 @@ TEST(uri_test, query_iterator_with_fragment) {
884907
EXPECT_EQ("c", query_it->first);
885908
EXPECT_EQ("d", query_it->second);
886909
}
910+
911+
TEST(uri_test, copy_assignment_bug_98) {
912+
network::uri original("file:///path/to/file.txt");
913+
914+
ASSERT_TRUE(original.has_scheme());
915+
ASSERT_FALSE(original.is_opaque());
916+
ASSERT_TRUE(original.has_host());
917+
ASSERT_TRUE(original.has_path());
918+
919+
network::uri instance;
920+
instance = original;
921+
922+
ASSERT_TRUE(instance.has_scheme());
923+
ASSERT_TRUE(instance.has_host());
924+
ASSERT_TRUE(instance.has_path());
925+
EXPECT_EQ("file", instance.scheme());
926+
EXPECT_EQ("", instance.host());
927+
EXPECT_EQ("/path/to/file.txt", instance.path());
928+
}
929+
930+
TEST(uri_test, copy_assignment_bug_98_2) {
931+
network::uri original("file:///path/to/file.txt?query=value#foo");
932+
933+
network::uri instance;
934+
instance = original;
935+
936+
ASSERT_TRUE(instance.has_scheme());
937+
ASSERT_TRUE(instance.has_path());
938+
ASSERT_TRUE(instance.has_query());
939+
ASSERT_TRUE(instance.has_fragment());
940+
EXPECT_EQ("file", instance.scheme());
941+
EXPECT_EQ("/path/to/file.txt", instance.path());
942+
EXPECT_EQ("query=value", instance.query());
943+
EXPECT_EQ("foo", instance.fragment());
944+
}
945+
946+
TEST(uri_test, copy_constructor_bug_98) {
947+
network::uri original("file:///path/to/file.txt?query=value#foo");
948+
949+
network::uri instance(original);
950+
951+
ASSERT_TRUE(instance.has_scheme());
952+
ASSERT_TRUE(instance.has_path());
953+
ASSERT_TRUE(instance.has_query());
954+
ASSERT_TRUE(instance.has_fragment());
955+
EXPECT_EQ("file", instance.scheme());
956+
EXPECT_EQ("/path/to/file.txt", instance.path());
957+
EXPECT_EQ("query=value", instance.query());
958+
EXPECT_EQ("foo", instance.fragment());
959+
}
960+
961+
TEST(uri_test, move_assignment_bug_98) {
962+
network::uri original("file:///path/to/file.txt?query=value#foo");
963+
964+
network::uri instance;
965+
instance = std::move(original);
966+
967+
ASSERT_TRUE(instance.has_scheme());
968+
ASSERT_TRUE(instance.has_path());
969+
ASSERT_TRUE(instance.has_query());
970+
ASSERT_TRUE(instance.has_fragment());
971+
EXPECT_EQ("file", instance.scheme());
972+
EXPECT_EQ("/path/to/file.txt", instance.path());
973+
EXPECT_EQ("query=value", instance.query());
974+
EXPECT_EQ("foo", instance.fragment());
975+
}
976+
977+
TEST(uri_test, move_constructor_bug_98) {
978+
network::uri original("file:///path/to/file.txt?query=value#foo");
979+
980+
network::uri instance(std::move(original));
981+
982+
ASSERT_TRUE(instance.has_scheme());
983+
ASSERT_TRUE(instance.has_path());
984+
ASSERT_TRUE(instance.has_query());
985+
ASSERT_TRUE(instance.has_fragment());
986+
EXPECT_EQ("file", instance.scheme());
987+
EXPECT_EQ("/path/to/file.txt", instance.path());
988+
EXPECT_EQ("query=value", instance.query());
989+
EXPECT_EQ("foo", instance.fragment());
990+
}
991+
992+
TEST(uri_test, http_copy_assignment_bug_98) {
993+
network::uri original("http://example.com/path/to/file.txt");
994+
995+
network::uri instance;
996+
instance = original;
997+
998+
ASSERT_TRUE(instance.has_scheme());
999+
ASSERT_TRUE(instance.has_path());
1000+
EXPECT_EQ("http", instance.scheme());
1001+
EXPECT_EQ("/path/to/file.txt", instance.path());
1002+
}

0 commit comments

Comments
 (0)