Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
varjolintu opened this issue Sep 26, 2022 · 3 comments
Open

Comments

@varjolintu
Copy link

varjolintu commented Sep 26, 2022

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

Steps to reproduce:

  • Have a String URL with username and password: https://username:password@example.com.
  • Create a new URL using Url::parse() from the String.
  • Use set_username() and set_password() to reset the values as empty strings.
  • .as_str() or .to_string() for the Url shows: https://:@example.com/.

Using version 2.3.1 of the crate.

@varjolintu
Copy link
Author

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
Copy link

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 #806

@qsantos
Copy link
Contributor

qsantos commented Mar 14, 2023

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 ?").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants