servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.31k stars 325 forks source link

URL with username/password removed creates invalid URL with .to_string() #796

Open varjolintu opened 2 years ago

varjolintu commented 2 years ago

When setting username/password to empty string value, : and @ chars should be removed from the URL when using to_string().

Steps to reproduce:

Using version 2.3.1 of the crate.

varjolintu commented 2 years ago

Same applies to query. If URL has a query and it is set to empty string using set_query(), printing the URL still shows ? at the end of the URL.

raccmonteiro commented 1 year ago

There's a difference between set_password(Some("")) and set_password(None), the first has an empty string as password and output of .as_str() is https://:@example.com/ as you mentioned, the later does not have a password and the output is https://example.com/. Check the new tests in this PR https://github.com/servo/rust-url/pull/806

qsantos commented 1 year ago

From the URL Standard, in URL serializing:

  1. If url includes credentials, then:
    1. Append url’s username to output.
    2. If url’s password is not the empty string, then append U+003A (:), followed by url’s password, to output.
    3. Append U+0040 (@) to output.

The first link of the quote leads to:

A URL includes credentials if its username or password is not the empty string.

So I think the set_password(Some("")) should have the same effect on the URL as set_password(None), and we should not leave an empty :@.

This is consistent with normalization, as seen in test_serialization and test_url_visualizer. There are also several examples in urltestdata.json (search for :@).


However, the standard also says:

  1. If url’s query is non-null, append U+003F (?), followed by url’s query, to output.

Note that the standard does make the distinction between “null” and “empty-string”. So I think set_query(Some("")) should keep the trailing ?.

This is consistent with examples of normalization in urltestdata.json (search for ?").