go: proposal: net/http.Client should allow omitting URLs from error strings

Context

When using a net/http.Client, especially with a context.Context that has a deadline, it’s not uncommon for the http.Client.Do() method to return an error that contains the full URL used when actioning the request. Unfortunately that URL may contain sensitive query parameters, or other things that make it unsafe for directly logging or forwarding elsewhere without first sanitizing the URL or removing it entirely.

There was a Gopher who recently ran into a case where they were almost writing sensitive URLs out to log files, but they managed to catch that in a pre-production environment. They could, at each http.Client.Do() call site, write logic to reformat the error value to not include the URL, but there are a few problems with that approach.

The first is that you’d run the risk of forgetting to add that protection somewhere, or a code change / addition in the future could omit it. In addition if you’re mutating the error value to remove the URL, you’ve made it so that callers cannot inspect that detail of the error if they wanted to use it to decide to handle the error in a specific way.

Proposal

Considering the above context, I’d like to propose that we extend the net/http.Client to optionally omit the URL from any and all errors that get generated when it tries to action a request. This would allow programmers to set the behavior on the client itself, mitigating the first risk of forgetting to filter the URL from the error at a specific call site. This would be accomplished by adding a bool field to the net/http.Client struct: OnErrorOmitURL. The zero value (false) for this field would result in the http.Client behaving like it does today, meaning this would not be a breaking change.

Because the errors returned from the http.Client.Do() method are of type net/url.Error, we will also need to extend that type to include a bool struct field, OmitURL, which results in the Error() method not including the URL in the error string it returns. This will address the second risk above as the URL will still be present in string form within the url.Error.URL field, but it won’t be implicitly included when the error value is printed or logged. Like the field on the net/http.Client, the zero value (false) will result in url.Error working like it does today, meaning this would also not be a breaking change.

About this issue

Original URL
State: open
Created 3 years ago
Reactions: 16
Comments: 15 (14 by maintainers)

Most upvoted comments

You can do something like this to redact the URLs without requiring any changes to net/http (playground link - won’t run on the playground because of #44822):

func RedactErrorURL(err error) {
	var ue *url.Error
	if errors.As(err, &ue) {
		ue.URL = "<redacted>"
	}
}

That works even when you don’t have control over the Client struct (which is pretty common in my experience) and can be used wherever is appropriate. You could use it directly after a http request, or after a wrapper function is done, or all the way down at where the error is actually being logged.

tmthrgd on Mar 9, 2021

This seems related to net/url.(*URL).Redacted. And, for that matter, net/http already tries to strip the password in a returned URL (https://go.googlesource.com/go/+/refs/heads/master/src/net/http/client.go#1003).

This seems like a reasonable idea to me if you want to write a proposal. Thanks.

ianlancetaylor on Mar 5, 2021

If we’re willing to permit net/http.Client.MutateErr to return a non-*url.Error error, then a redaction function could return an error that wraps the original *url.Error.

type redactedURLError url.Error

func (e *redactedURLError) Unwrap() error { return (*url.Error)(e) }
func (e *redactedURLError) Error() string { return fmt.Sprintf("%s: %s", e.Op, e.Err) }

func RedactURLError(err error) error {
  if e, ok := err.(*url.Error); ok {
    return (*redactedURLError)(e)
  }
  return e
}

client.MutateError = RedactURLError

This permits inspecting the original URL (by recovering the url.Error with errors.As), while redacting it from the error text.

neild on Jul 28, 2021