Applied fix for #116 (#117)

glynos · web-flow · commit 6be6c24b45d3 · 2018-10-13T13:45:07.000+02:00
diff --git a/include/network/uri/detail/uri_parts.hpp b/include/network/uri/detail/uri_parts.hpp
@@ -58,6 +58,13 @@ struct hierarchical_part {
   optional<uri_part> host;
   optional<uri_part> port;
   optional<uri_part> path;
+
+  void clear() {
+    user_info = nullopt;
+    host = nullopt;
+    port = nullopt;
+    path = nullopt;
+  }
 };
 
 struct uri_parts {
@@ -67,6 +74,13 @@ struct uri_parts {
   hierarchical_part hier_part;
   optional<uri_part> query;
   optional<uri_part> fragment;
+
+  void clear() {
+    scheme = nullopt;
+    hier_part.clear();
+    query = nullopt;
+    fragment = nullopt;
+  }
 };
 }  // namespace detail
 }  // namespace network
diff --git a/src/uri.cpp b/src/uri.cpp
@@ -196,7 +196,9 @@ void uri::swap(uri &other) noexcept {
   other.uri_view_ = other.uri_;
 
   const auto this_parts = uri_parts_;
+  uri_parts_.clear();
   detail::advance_parts(uri_view_, uri_parts_, other.uri_parts_);
+  other.uri_parts_.clear();
   detail::advance_parts(other.uri_view_, other.uri_parts_, this_parts);
 }
 
diff --git a/test/uri_builder_test.cpp b/test/uri_builder_test.cpp
@@ -775,3 +775,26 @@ TEST(builder_test, scheme_and_absolute_path) {
   ASSERT_EQ("foo", builder.uri().scheme());
   ASSERT_EQ("/bar", builder.uri().path());
 }
+
+TEST(builder_test, assignment_operator_bug_116) {
+  // https://github.com/cpp-netlib/uri/issues/116
+  network::uri a("http://a.com:1234");
+  ASSERT_TRUE(a.has_port());
+
+  const network::uri b("http://b.com");
+  ASSERT_FALSE(b.has_port());
+
+  a = b;
+  ASSERT_FALSE(a.has_port()) << a.string();
+}
+
+TEST(builder_test, construct_from_uri_bug_116) {
+  // https://github.com/cpp-netlib/uri/issues/116
+  network::uri a("http://a.com:1234");
+  const network::uri b("http://b.com");
+  a = b;
+
+  network::uri_builder ub(a);  // ASAN reports heap-use-after-free here
+  const network::uri c(ub.uri());
+  ASSERT_FALSE(c.has_port()) << c.string();
+}
diff --git a/test/uri_test.cpp b/test/uri_test.cpp
@@ -1058,3 +1058,14 @@ TEST(uri_test, uri_has_host_bug_88_2) {
 
   EXPECT_EQ("example.com", instance.host().to_string());
 }
+
+TEST(uri_test, assignment_operator_bug_116) {
+  network::uri a("http://a.com:1234");
+  ASSERT_TRUE(a.has_port());
+
+  const network::uri b("http://b.com");
+  ASSERT_FALSE(b.has_port());
+
+  a = b;
+  ASSERT_FALSE(a.has_port()) << a.string() << ", " << a.port();
+}

Original file line number	Diff line number	Diff line change
`@@ -196,7 +196,9 @@ void uri::swap(uri &other) noexcept {`
`196`	`196`	`other.uri_view_ = other.uri_;`
`197`	`197`
`198`	`198`	`const auto this_parts = uri_parts_;`
	`199`	`+ uri_parts_.clear();`
`199`	`200`	`detail::advance_parts(uri_view_, uri_parts_, other.uri_parts_);`
	`201`	`+ other.uri_parts_.clear();`
`200`	`202`	`detail::advance_parts(other.uri_view_, other.uri_parts_, this_parts);`
`201`	`203`	`}`
`202`	`204`