boost.url review

From: Christopher Bläsius (christopher.blaesius_at_[hidden])
Date: 2022-08-22 17:25:16

Hi,

I'm not a boost developer, but as someone who has to deal with URLs (for 
calling or implementing Web-APIs) at his job frequently, I want to give 
my two cents as a most likely user of this library. I hope this is OK (I 
won't give a recommendation to accept or reject though).

I spent about 2.5h integrating the library into our program and tried 
some simple parsing and constructing of urls. I used MSVC2022 which 
worked without problems.

> - Does this library bring real benefit to C++ developers for real  > world use-case? Yes definitely. I believe probably every programmer 
has written a (mostly crappy) URL-parser before, and I would be more 
than happy to replace our parser with this library.

> - Do you have an application for this library?
Yes. Besides parsing incoming HTTP-Requests, I would use it to 
programmatically construct urls to call external Web-APIs.

One of the more common scenarios in our applications is the construction 
of urls after the user provided some login credentials and often 
hosts/port combinations. Also a prefix path is often needed (for example 
to differentiate between a demo or beta service and the real production 
api). I feel like the current  API design lacks a bit in this regard.

To construct a new url i did the following:

 Â Â Â  auto url = boost::urls::url{};
 Â Â Â  url.set_scheme("https")
 Â Â Â Â  .set_host("special-api.com")
 Â Â Â Â  .set_port("443")
 Â Â Â Â  .set_path("/prefix/api/v1/");
 Â Â Â  url.segments().push_back("applications");
 Â Â Â  url.segments().push_back(applicationId);
 Â Â Â  url.segments().push_back("devices");
 Â Â Â  url.params().append("since", "2022-01-01");

The url::set_... methods seem to follow a fluent api design which is 
nice, but unfortunately they return url_base&. So I can't construct a 
url inside a function call like:

httpClientGet(url{}.set_host(host).set_path("some/path"))

Being able to add segments is great. Personally I'm a big fan of 
overloading the /-operator to append paths like in the filesystem 
library, because I think this results in more readable code. The example 
of above could become:

httpClientGet(url{host} / "some/path")

I really like that the library offers different parse_ functions, so an 
user can choose the appropriate one depending on the context.

But it is a shame that there are no semantic types to go along with 
them. As mentioned above we often have the case that we have to 
construct a url by combining a prefix path with the actual path of the 
resource. As this prefix path has to be absolute and the resource path 
relative I want to use 2 distinct types for them. It seems to me, the 
resolve() function has a similar problem. The documentation states that 
the base url has to satisfy the absolute-URI grammar. Wouldn't it be 
great to have a distinct type for this?

BTW why is there only an authority_view and not an owning container? 
Seems inconsistent to me.

Its nice that there are functions to manually do percentage encoding, 
but I would prefer (additional) simpler functions which just return a 
std::string.

It would be nice if the library also provided the default port numbers 
for the schemes. I know the documentation states that this is not part 
of the URL-protocol, but I gonna need that information if I use the library.

> - Is the documentation helpful and clear?
Mostly yes. Reading the "Quick Look" page is enough to get started with 
using the library which I appreciate. I also like the help card, which 
would be greatly improved by adding links to the reference pages.

I skipped / skimmed the Customization and Concept pages.

I would recommend not to start your example page with a 700 lines 
implementation of magnet links, that's super intimidating.

Something I missed was a simple example showing that the parse functions 
also accept unknown schemes like opc.tcp:// for example.

Thanks a lot for all your work on providing useful high quality 
libraries for everyone!

Best Regards,
Chris