`set_port`, `set_username` & `set_password` should check for a value before returning `Err(())` about rust-url HOT 7 OPEN

Why do you expect this?

from rust-url.

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());
    }
}

from rust-url.

Or you check for the empty string before calling set_username?

from rust-url.

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.

from rust-url.

We're bound to whatever the URL specification says. I'd have to check whether we can change this

from rust-url.

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.

from rust-url.

This also applies to set_port

from rust-url.