Open chanced opened 1 year ago
Why do you expect this?
I need to wrap Url
for a Uri
type. To do so, I need a method that allows for setting the authority. It would be nice not to have to ignore errors when attempting to set an empty value when it should not be applicable.
My workaround:
if u.set_username(authority.username().unwrap_or_default())
.is_err()
{
// the url crate doesn't check for empty values before returning `Err(())`
// https://github.com/servo/rust-url/issues/844
let username = authority.username().unwrap_or_default();
if !username.is_empty() {
return Err(AuthorityError::UsernameNotAllowed(username.to_string()).into());
}
}
if u.set_password(authority.password()).is_err() {
let password = authority.password().unwrap_or_default();
if !password.is_empty() {
return Err(AuthorityError::PasswordNotAllowed(password.to_string()).into());
}
}
u.set_host(authority.host())?;
if u.set_port(authority.port()).is_err() {
if let Some(port) = authority.port() {
return Err(AuthorityError::PortNotAllowed(port).into());
}
}
Or you check for the empty string before calling set_username?
Sure, but that may be a valid value (as in, unsetting the username). Arguably, I could perform the same checks (!url.has_host() || url.host() == Some(Host::Domain("")) || url.scheme() == "file"
) but that leaves my side of things brittle. If additional checks are added to the url crate, I need to mirror those.
We're bound to whatever the URL specification says. I'd have to check whether we can change this
I think you should be able to.
This only changes whether an error is returned in the event that someone attempts to set an empty port/username/password when the port/username/password cannot be set.
On the happy path, for example when an empty string is attempted to be set for the username of a file://
url, the Url
would not be altered anyhow. The same goes for password. The call effectively becomes a no-op.
If a non-empty value is provided for either and they are not allowed, the current logic of returning an Err(())
is maintained.
This also applies to set_port
The methods
set_port
,set_username
andset_password
all fail early if called when the url cannot have a port/username/password:I believe they should first check for the existence of a value (or empty string, in the case of
set_username
) before returning an error.