Security review fixes (v3)

include/boost/url/detail/impl/segments_iter_impl.hpp

@@ -99,6 +99,9 @@ void
 segments_iter_impl::
 update() noexcept
 {
+    BOOST_ASSERT(
+        pos == 0 ||
+        ref.data()[pos - 1] == '/');
     auto const end = ref.end();
     char const* const p0 =
         ref.data() + pos;
@@ -182,39 +185,27 @@ decrement() noexcept
     --index;
     if(index == 0)
     {
-        next = pos;
         pos = path_prefix(ref.buffer());
         decoded_prefix = pos;
-        s_ = core::string_view(
-            ref.data() + pos,
-            next - pos);
+        update();
         BOOST_ASSERT(! s_.ends_with('/'));
         return;
     }
+    // scan backwards to find the '/' before
+    // the previous segment
     auto const begin = ref.data() +
         path_prefix(ref.buffer());
-    next = pos;
-    auto p = ref.data() + next;
-    auto const p1 = p;
+    auto p = ref.data() + pos;
     BOOST_ASSERT(p != begin);
-    dn = 0;
     while(p != begin)
     {
         --p;
         if(*p == '/')
-        {
-            ++dn;
             break;
-        }
-        if(*p == '%')
-            dn += 2;
     }
-    dn = p1 - p - dn;
-    pos = p - ref.data();
-    // keep decoded_prefix consistent with new pos
-    // (already adjusted above)
-    s_ = make_pct_string_view_unsafe(
-        p + 1, p1 - p - 1, dn);
+    pos = p - ref.data() + 1;
+    update();
+    --pos;
 }
 
 } // detail

include/boost/url/grammar/string_view_base.hpp

@@ -159,7 +159,7 @@ class string_view_base
     */
     explicit
     operator
-    std::string() const noexcept
+    std::string() const
     {
         return std::string(s_);
     }

include/boost/url/impl/url_view.hpp

@@ -67,9 +67,10 @@ persist() const
     auto p = std::allocate_shared<T>(
         detail::over_allocator<T, Alloc>(
             size(), a), url_view(impl()));
-    std::memcpy(
-        reinterpret_cast<char*>(
-            p.get() + 1), data(), size());
+    if(size())
+        std::memcpy(
+            reinterpret_cast<char*>(
+                p.get() + 1), data(), size());
     return p;
 }
 

include/boost/url/param.hpp

@@ -356,7 +356,7 @@ struct param
     param(
         core::string_view key,
         core::string_view value,
-        bool has_value) noexcept
+        bool has_value)
         : key(key)
         , value(has_value
             ? value

test/unit/grammar/string_view_base.cpp

@@ -16,11 +16,40 @@ namespace boost {
 namespace urls {
 namespace grammar {
 
+// Minimal derived class for testing
+struct test_string_view
+    : string_view_base
+{
+    explicit
+    test_string_view(
+        core::string_view s) noexcept
+        : string_view_base(s)
+    {
+    }
+};
+
 struct string_view_base_test
 {
     void
     run()
     {
+        // operator std::string() allocates
+        static_assert(
+            !noexcept(std::string(
+                std::declval<test_string_view const&>())),
+            "operator std::string() must not be noexcept");
+
+        // Verify the conversion still works correctly
+        {
+            test_string_view sv("hello");
+            std::string s = static_cast<std::string>(sv);
+            BOOST_TEST(s == "hello");
+        }
+        {
+            test_string_view sv("");
+            std::string s = static_cast<std::string>(sv);
+            BOOST_TEST(s.empty());
+        }
     }
 };
 

test/unit/param.cpp

@@ -44,6 +44,14 @@ struct param_test
     // cheap, loses pct-validation
     BOOST_CORE_STATIC_ASSERT(std::is_constructible<param_view, param_pct_view>::value);
 
+    // param allocates std::string members
+    BOOST_CORE_STATIC_ASSERT(
+        !std::is_nothrow_constructible<
+            param,
+            core::string_view,
+            core::string_view,
+            bool>::value);
+
     // expensive constructions
     BOOST_CORE_STATIC_ASSERT(std::is_constructible<param_pct_view, param>::value);
     BOOST_CORE_STATIC_ASSERT(std::is_constructible<param_pct_view, param_view>::value);
@@ -163,6 +171,18 @@ struct param_test
             BOOST_TEST_GE(qp.value.capacity(), 100);
         }
 
+        // param(string_view, string_view, bool) - 3-arg ctor
+        {
+            {
+                param qp("key", "value", true);
+                check(qp, "key", "value", true);
+            }
+            {
+                param qp("key", "value", false);
+                check(qp, "key", "", false);
+            }
+        }
+
         // operator->
         {
             param qp("key", "value");

test/unit/segments_encoded_base.cpp

@@ -200,6 +200,64 @@ struct segments_encoded_base_test
         }
     }
 
+    void
+    testDecrementDecodedLength()
+    {
+        // Regression: decrement to index 0 must
+        // recompute dn so that a subsequent
+        // increment gets decoded_prefix right.
+        {
+            // percent-encoded first segment
+            auto rv = parse_uri_reference(
+                "/a%20b/c");
+            BOOST_TEST(rv.has_value());
+            auto const& ps =
+                rv->encoded_segments();
+            // iterate forward then backward
+            auto it = ps.begin();
+            BOOST_TEST_EQ(*it, "a%20b");
+            BOOST_TEST_EQ(
+                it->decoded_size(), 3u);
+            ++it;
+            BOOST_TEST_EQ(*it, "c");
+            // decrement back to first segment
+            --it;
+            BOOST_TEST_EQ(*it, "a%20b");
+            BOOST_TEST_EQ(
+                it->decoded_size(), 3u);
+            // increment again: decoded_prefix
+            // must be consistent
+            ++it;
+            BOOST_TEST_EQ(*it, "c");
+            BOOST_TEST_EQ(
+                it->decoded_size(), 1u);
+        }
+        {
+            // multiple percent-encoded segments
+            auto rv = parse_uri_reference(
+                "/%25a/%20b/c");
+            BOOST_TEST(rv.has_value());
+            auto const& ps =
+                rv->encoded_segments();
+            auto it = ps.end();
+            --it;
+            BOOST_TEST_EQ(*it, "c");
+            --it;
+            BOOST_TEST_EQ(*it, "%20b");
+            BOOST_TEST_EQ(
+                it->decoded_size(), 2u);
+            --it;
+            BOOST_TEST_EQ(*it, "%25a");
+            BOOST_TEST_EQ(
+                it->decoded_size(), 2u);
+            // roundtrip back to end
+            ++it;
+            BOOST_TEST_EQ(*it, "%20b");
+            ++it;
+            BOOST_TEST_EQ(*it, "c");
+        }
+    }
+
     void
     testRange()
     {
@@ -223,6 +281,10 @@ struct segments_encoded_base_test
         check( "images/cat-pic.gif", { "images", "cat-pic.gif" });
         check( "/fast//query", { "fast", "", "query" });
         check( "fast//",  { "fast", "", "" });
+        // percent-encoded first segment (exercises
+        // decrement to index 0 with dn recalculation)
+        check( "/a%20b/c", { "a%20b", "c" });
+        check( "/%25x/%20y/z", { "%25x", "%20y", "z" });
     }
 
     void
@@ -269,6 +331,8 @@ struct segments_encoded_base_test
     void
     run()
     {
+        testObservers();
+        testDecrementDecodedLength();
         testRange();
         testJavadocs();
     }

test/unit/url_view.cpp

@@ -217,6 +217,16 @@ class url_view_test
         }
         }
 
+        // persist() with null data() and zero size()
+        {
+            url_view u;
+            BOOST_TEST(u.empty());
+            auto sp = u.persist();
+            BOOST_TEST(sp->empty());
+            BOOST_TEST_EQ(sp->size(), 0u);
+            BOOST_TEST_EQ(sp->buffer(), "");
+        }
+
         // operator core::string_view()
         {
             auto const f = []( core::string_view ) {};