From 60d11815351c971d033d230fb9ee494e4779fcea Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 14 Sep 2026 12:37:23 +0200 Subject: [PATCH] [client] Stop the URL redaction from eating the prose after the URL The class `[^\s"']+` ran past every delimiter that is not whitespace or a quote, so a URL followed by `)`, `>` or a backtick took the closing character and the words after it into the match, and everything past the small TrimRight set was dropped from the message: (see https://upload.example.com/x?t=1) for details -> (see https://upload.example.com for details Stop the match at those delimiters and leave TrimRight to sentence punctuation. Three cases added. Reported by cubic on #7514. --- client/internal/debug/upload.go | 8 ++++++-- client/internal/debug/upload_redact_test.go | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/client/internal/debug/upload.go b/client/internal/debug/upload.go index d3a1ca9cf..30aa73b91 100644 --- a/client/internal/debug/upload.go +++ b/client/internal/debug/upload.go @@ -178,8 +178,12 @@ func getURLHash(url string) string { return fmt.Sprintf("%x", sha256.Sum256([]byte(url))) } -// urlInText matches an absolute http(s) URL inside a free-form message. -var urlInText = regexp.MustCompile(`https?://[^\s"']+`) +// urlInText matches an absolute http(s) URL inside a free-form message. The +// class stops at the delimiters an error message wraps a URL in — quotes, +// backticks, angle brackets, parens and braces — so the match does not run past +// the URL and swallow the prose after it. TrimRight below then drops trailing +// sentence punctuation, which a bare URL at the end of a clause picks up. +var urlInText = regexp.MustCompile("https?://[^\\s\"'`<>\\[\\]{}()]+") // redactedError keeps the original error reachable for errors.Is/As while // presenting a message with every URL cut down to scheme://host. diff --git a/client/internal/debug/upload_redact_test.go b/client/internal/debug/upload_redact_test.go index 896201fea..bea5227e9 100644 --- a/client/internal/debug/upload_redact_test.go +++ b/client/internal/debug/upload_redact_test.go @@ -38,7 +38,7 @@ func TestRedactURLsInError(t *testing.T) { { // The presigned PUT URL is the one that carries credentials. name: "presigned URL loses its credentials", - err: errors.New(`upload failed: Put "https://bucket.s3.amazonaws.com/k?X-Amz-Signature=abc123&X-Amz-Credential=AKIA": timeout`), + err: errors.New(`upload failed: Put "https://bucket.s3.amazonaws.com/k?X-Amz-Signature=abc123&X-Amz-Credential=AKIA": timeout`), want: `upload failed: Put "https://bucket.s3.amazonaws.com": timeout`, }, { @@ -51,6 +51,23 @@ func TestRedactURLsInError(t *testing.T) { err: errors.New(`redirect from https://a.example.com/x?t=1 to https://b.example.com/y?t=2`), want: `redirect from https://a.example.com to https://b.example.com`, }, + { + // The match must stop at the delimiter, not run on and eat the + // words after it. + name: "closing paren and the prose after it survive", + err: errors.New(`(see https://upload.example.com/x?t=1) for details`), + want: `(see https://upload.example.com) for details`, + }, + { + name: "angle brackets survive", + err: errors.New(`tried and failed`), + want: `tried and failed`, + }, + { + name: "backticks survive", + err: errors.New("use `https://b.example.com/p` instead"), + want: "use `https://b.example.com` instead", + }, } for _, tc := range tests {