diff --git a/modules/util/url.go b/modules/util/url.go index 2fde9890eb..ea9fe79c2e 100644 --- a/modules/util/url.go +++ b/modules/util/url.go @@ -27,22 +27,20 @@ func SanitizeURL(s string) (string, error) { return u.String(), nil } -// SanitizeURLForLog returns a redacted form of a URL safe to include in -// log lines. It strips userinfo (e.g. https://user:pass@…), the query -// string (which may contain signed-URL credentials such as AWS S3 / GCS / -// Cloudinary signatures), and the fragment, leaving only scheme+host+path. -// On a parse failure the placeholder "" is returned to -// avoid leaking the raw URL into logs. +// StripUrl returns the scheme, host, and path portions of s with userinfo, +// query string, and fragment removed. Intended for logging URLs whose +// userinfo or query string may carry credentials (e.g. https://user:pass@… +// or signed S3/GCS/Cloudinary URLs whose signatures live in the query +// string). Returns "" if s cannot be parsed. // -// Unlike SanitizeURL this is intended exclusively for logging: callers -// that still need to USE the URL (mirroring, indexing, migrations, etc.) -// should keep using SanitizeURL because they need the query string -// preserved. -func SanitizeURLForLog(s string) string { +// Unlike SanitizeURL (which only strips userinfo and is used by callers +// such as mirroring/indexing/migrations that still need the query string +// to actually use the URL), StripUrl is for logging only. +func StripUrl(s string) string { u, err := url.Parse(s) if err != nil { return "" } - redacted := url.URL{Scheme: u.Scheme, Host: u.Host, Path: u.Path} - return redacted.String() + stripped := url.URL{Scheme: u.Scheme, Host: u.Host, Path: u.Path} + return stripped.String() } diff --git a/modules/util/url_test.go b/modules/util/url_test.go index e922d0cf28..4bb06ecde3 100644 --- a/modules/util/url_test.go +++ b/modules/util/url_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestSanitizeURLForLog(t *testing.T) { +func TestStripUrl(t *testing.T) { cases := []struct { name string in string @@ -54,7 +54,7 @@ func TestSanitizeURLForLog(t *testing.T) { } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - assert.Equal(t, c.want, SanitizeURLForLog(c.in)) + assert.Equal(t, c.want, StripUrl(c.in)) }) } } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 7b41ae53d0..1432662d77 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -308,7 +308,7 @@ func oauth2UpdateAvatarIfNeed(ctx *context.Context, rawURL string, u *user_model } // Compute a redacted URL for log lines BEFORE issuing the request, so we // never accidentally log signed-URL query parameters or userinfo. - logURL := util.SanitizeURLForLog(rawURL) + logURL := util.StripUrl(rawURL) // Bind the outbound fetch to the inbound request context so the download // is cancelled if the user navigates away / aborts login, and so any