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
net/url: in Parse, strip password when copying input into Error #53993
Comments
Maybe we should add a
cc @neild |
I don't believe SafeParse is the answer. Perhaps we should do something about this, but perhaps not. There are lots of ways to provide malformed input that might leak passwords. What if someone does url.Parse("mysql://user:password2example.com:3306/db") instead? |
Is there another way to avoid getting your password leaked and still use this library?
https://play.golang.com/p/9AAL2wAZ0_t There does seem to be some precedent of having specific parses for special cases: You mention as an example
Assumptions seemingly in line with this section of the RFC that the url package follows.:
Given what is said on the user information section:
It is clear that it is not fair to assume a |
@ekeric13 You should not print the original error, if you worry about password leak. You can unwrap the original parsed, err := url.Parse(rawURL)
if err != nil {
if uerr := new(url.Error); errors.As(err, &uerr) {
return nil, uerr.Err
}
return nil, errors.New("invalid URL")
} But still, there is no guarantee that it's impossible to manage parser to get the password from the input. |
thanks @tdakkota . That is a more refined approach of what I am currently doing, which is avoiding printing the error. But I didn't realize you can pull out only part of the error message. |
@rsc This is true but why return the As a workaround I do this in my code but I can't guarantee other packages are as responsible that use // Validate the URL.
_, err := url.Parse(rawURL)
if err != nil {
// Unwrap the error to get the original error.
// So that creds are not in the returned error.
err := errors.Unwrap(err)
return "", fmt.Errorf("cannot parse url: %w", err)
} |
If you don't want to information about the (un)parsed URL in the error, then the simplest and most robust thing to do is to replace the error returned by
Perhaps |
In Go, errors include information about what failed. Effective Go discusses why this is a good thing (see the part about the error returned from os.Open - the same considerations apply to strconv.Atoi or url.Parse or any other function). |
Slightly different iteration of this issue: #24572
Just using a different string
What version of Go are you using (
go version
)?go1.18.4
Does this issue reproduce with the latest release?
With the latest stable release
What operating system and processor architecture are you using (
go env
)?Reproduced in go playground
What did you do?
https://play.golang.com/p/8GXuAZ66w3V
What did you expect to see?
What did you see instead?
The text was updated successfully, but these errors were encountered: