boostorg / url

Boost.URL is a library for manipulating Uniform Resource Identifiers (URIs) and Locators (URLs).
https://www.boost.org/doc/libs/master/libs/url/index.html
Boost Software License 1.0
185 stars 50 forks source link

resolve fails when using assigned-to url_view #872

Closed mgroenhoff closed 1 week ago

mgroenhoff commented 1 week ago

I am running into an issue where first defining an empty url_view and later assigning it from an url makes resolve return an error (case 2) whereas constructing an url_view with an url works fine (case 1):

#include <boost/url.hpp>
#include <iostream>

void test_resolve(boost::url_view base, boost::url_view ref) {
    boost::url dest;
    auto resolve_result = boost::urls::resolve(base, ref, dest);
    if (resolve_result) {
        std::cerr << "ok: " << dest << std::endl;
    } else {
        std::cerr << "error: " << resolve_result.error().message() << std::endl;
    }
}

int main() {
    boost::url base{"https://127.0.0.1"};
    boost::url_view ref = "/foo";

    // Case 1
    boost::url_view base_view1{base}; // constructing works
    assert(base_view1.has_scheme()); // ok. doesn't assert
    test_resolve(base_view1, ref); // outputs "ok"

    // Case 2
    boost::url_view base_view2;
    base_view2 = base; // assign instead of construct
    assert(base_view2.has_scheme()); // ok. doesn't assert
    test_resolve(base_view2, ref); // outputs: "error: not_a_base" even though the assert above doesn't fire

    return 0;
}
alandefreitas commented 1 week ago

I'm trying to replicate it but

resolve(base_view1, ref); 

Does this function exist?

The free function has 3 arguments: https://www.boost.org/doc/libs/master/doc/antora/url/reference/boost/urls/resolve.html

And the member function has only 1: https://www.boost.org/doc/libs/master/doc/antora/url/reference/boost/urls/url_base/resolve.html

It can't be

base_view1.resolve(ref); 

either because views don't have the resolve member function.

mgroenhoff commented 1 week ago

@alandefreitas It is the function in the example above main which is simply a small wrapper. Maybe I should've given it another name to avoid confusion.

edit: renamed resolve to test_resolve

alandefreitas commented 1 week ago

Oh... Let me replicate that without the wrapper then.

mgroenhoff commented 1 week ago

I tried to make the example even smaller and it seems I don't even need the resolve. Here is a smaller example where the last assert goes off:

#include <boost/url.hpp>
#include <cassert>

int main() {
    boost::url base{"https://127.0.0.1"};

    // Case 1
    {
        boost::url_view base_view{base}; // construct view of base
        assert(base_view.has_scheme()); // ok. doesn't assert

        boost::url base_copy{base_view}; // copy from view of base
        assert(base_copy.has_scheme()); // ok. doesn't assert
    }

    // Case 2
    {
        boost::url_view base_view; // construct empty view
        base_view = base; // assign to be a view of base
        assert(base_view.has_scheme());

        boost::url base_copy{base_view}; // copy from view of base
        assert(base_copy.has_scheme()); // error: asserts
    }

    return 0;
}
alandefreitas commented 1 week ago

Thanks. I replicated it as:

        // issue #872
        {
            url base{"https://127.0.0.1"};
            url_view ref = "/foo";

            // url_view(url)
            {
                url_view base_view{base};
                BOOST_TEST(base_view.has_scheme());
                url dest;
                auto res = resolve(base_view, ref, dest);
                BOOST_TEST(res);
                BOOST_TEST(dest.has_scheme());
                BOOST_TEST_EQ(dest.buffer(), "https://127.0.0.1/foo");
            }

            // url_view::operator=(url)
            {
                url_view base_view;
                base_view = base;
                BOOST_TEST(base_view.has_scheme());
                url dest;
                auto res = resolve(base_view, ref, dest);
                BOOST_TEST(res);
                BOOST_TEST(dest.has_scheme());
                BOOST_TEST_EQ(dest.buffer(), "https://127.0.0.1/foo");
            }
        }
mgroenhoff commented 1 week ago

Thanks for fixing this so quickly!