Merge pull request #158 from dany74q/danny/redacted-url-in-logs

Redacted URL in logs / errors

Author: manicminer

client.go

@@ -658,9 +658,9 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 	if logger != nil {
 		switch v := logger.(type) {
 		case LeveledLogger:
-			v.Debug("performing request", "method", req.Method, "url", req.URL)
+			v.Debug("performing request", "method", req.Method, "url", redactURL(req.URL))
 		case Logger:
-			v.Printf("[DEBUG] %s %s", req.Method, req.URL)
+			v.Printf("[DEBUG] %s %s", req.Method, redactURL(req.URL))
 		}
 	}
 
@@ -715,9 +715,9 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 		if err != nil {
 			switch v := logger.(type) {
 			case LeveledLogger:
-				v.Error("request failed", "error", err, "method", req.Method, "url", req.URL)
+				v.Error("request failed", "error", err, "method", req.Method, "url", redactURL(req.URL))
 			case Logger:
-				v.Printf("[ERR] %s %s request failed: %v", req.Method, req.URL, err)
+				v.Printf("[ERR] %s %s request failed: %v", req.Method, redactURL(req.URL), err)
 			}
 		} else {
 			// Call this here to maintain the behavior of logging all requests,
@@ -753,7 +753,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 
 		wait := c.Backoff(c.RetryWaitMin, c.RetryWaitMax, i, resp)
 		if logger != nil {
-			desc := fmt.Sprintf("%s %s", req.Method, req.URL)
+			desc := fmt.Sprintf("%s %s", req.Method, redactURL(req.URL))
 			if resp != nil {
 				desc = fmt.Sprintf("%s (status: %d)", desc, resp.StatusCode)
 			}
@@ -818,11 +818,11 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 	// communicate why
 	if err == nil {
 		return nil, fmt.Errorf("%s %s giving up after %d attempt(s)",
-			req.Method, req.URL, attempt)
+			req.Method, redactURL(req.URL), attempt)
 	}
 
 	return nil, fmt.Errorf("%s %s giving up after %d attempt(s): %w",
-		req.Method, req.URL, attempt, err)
+		req.Method, redactURL(req.URL), attempt, err)
 }
 
 // Try to read the response body so we can reuse this connection.
@@ -903,3 +903,17 @@ func (c *Client) StandardClient() *http.Client {
 		Transport: &RoundTripper{Client: c},
 	}
 }
+
+// Taken from url.URL#Redacted() which was introduced in go 1.15.
+// We can switch to using it directly if we'll bump the minimum required go version.
+func redactURL(u *url.URL) string {
+	if u == nil {
+		return ""
+	}
+
+	ru := *u
+	if _, has := ru.User.Password(); has {
+		ru.User = url.UserPassword(ru.User.Username(), "xxxxx")
+	}
+	return ru.String()
+}

client_test.go

@@ -481,17 +481,32 @@ func TestClient_Do_fails(t *testing.T) {
 	}))
 	defer ts.Close()
 
+	serverUrlWithBasicAuth, err := url.Parse(ts.URL)
+	if err != nil {
+		t.Fatalf("failed parsing test server url: %s", ts.URL)
+	}
+	serverUrlWithBasicAuth.User = url.UserPassword("user", "pasten")
+
 	tests := []struct {
+		url  string
 		name string
 		cr   CheckRetry
 		err  string
 	}{
 		{
+			url:  ts.URL,
 			name: "default_retry_policy",
 			cr:   DefaultRetryPolicy,
 			err:  "giving up after 3 attempt(s)",
 		},
 		{
+			url:  serverUrlWithBasicAuth.String(),
+			name: "default_retry_policy_url_with_basic_auth",
+			cr:   DefaultRetryPolicy,
+			err:  redactURL(serverUrlWithBasicAuth) + " giving up after 3 attempt(s)",
+		},
+		{
+			url:  ts.URL,
 			name: "error_propagated_retry_policy",
 			cr:   ErrorPropagatedRetryPolicy,
 			err:  "giving up after 3 attempt(s): unexpected HTTP status 500 Internal Server Error",
@@ -508,15 +523,15 @@ func TestClient_Do_fails(t *testing.T) {
 			client.RetryMax = 2
 
 			// Create the request
-			req, err := NewRequest("POST", ts.URL, nil)
+			req, err := NewRequest("POST", tt.url, nil)
 			if err != nil {
 				t.Fatalf("err: %v", err)
 			}
 
 			// Send the request.
 			_, err = client.Do(req)
 			if err == nil || !strings.HasSuffix(err.Error(), tt.err) {
-				t.Fatalf("expected giving up error, got: %#v", err)
+				t.Fatalf("expected %#v, got: %#v", tt.err, err)
 			}
 		})
 	}