set_port, set_username & set_password should check for a value before returning Err(())
#844
Comments
|
Why do you expect this? |
|
I need to wrap 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 ( |
|
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 If a non-empty value is provided for either and they are not allowed, the current logic of returning an |
chanced
changed the title
set_username & set_password should check for a value before returning Err(())set_port, set_username & set_password should check for a value before returning Err(())
|
This also applies to |
The methods
set_port,set_usernameandset_passwordall 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.pub fn set_username(&mut self, username: &str) -> Result<(), ()> { // has_host implies !cannot_be_a_base if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" { + if username.is_empty() { + return Ok(()); + } return Err(()); }pub fn set_password(&mut self, password: Option<&str>) -> Result<(), ()> { // has_host implies !cannot_be_a_base if !self.has_host() || self.host() == Some(Host::Domain("")) || self.scheme() == "file" { + if password.is_none() || password == Some("") { + return Ok(()); + } return Err(()); }The text was updated successfully, but these errors were encountered: