oauthex: parse WWW-Authenticate in linear time

splitChallenges looked ahead from every comma by trimming and scanning
the rest of the header, so parsing took time quadratic in the header
length. A header made of 100 KB of commas took about 5.7s to parse, and
the work does not observe context cancellation.

Compute the lookahead once per run of commas and blanks, and read only
the next token to decide whether a comma starts an auth-param
(startsWithAuthParam). The result is unchanged except for a comma inside
the name before the '=', such as `x, a,b=c`: a token cannot contain a
comma (RFC 9110, section 5.6.2), so that comma now ends the challenge.

Add cases to TestSplitChallenges and TestParseWWWAuthenticateLinearTime,
which checks that a 100 KB run of commas parses within a deadline.

Fixes #1455

Author: jyotir-aditya

oauthex/resource_meta.go

@@ -180,6 +180,11 @@ func splitChallenges(header string) ([]string, error) {
 	inQuotes := false
 	escaped := false
 	start := 0
+	// Every comma in a run of commas and blanks sees the same text after the
+	// run, so the lookahead is computed once per run. Rescanning per comma
+	// makes a long header take quadratic time.
+	runEnd := 0
+	runIsParam := false
 	for i, r := range header {
 		if escaped {
 			// The previous backslash began a quoted-pair, so this character is
@@ -198,20 +203,13 @@ func splitChallenges(header string) ([]string, error) {
 			// This is a potential challenge separator.
 			// A new challenge does not start with `key=value`.
 			// We check if the part after the comma looks like a parameter.
-			lookahead := trimEmptyListElements(header[i+1:])
-			eqPos := strings.Index(lookahead, "=")
-
-			isParam := false
-			if eqPos > 0 {
-				// Check if the part before '=' is a single token (no spaces).
-				// Whitespace between the token and '=' is allowed (BWS, RFC 9110 section 11.2).
-				token := strings.TrimRight(lookahead[:eqPos], " \t")
-				if strings.IndexFunc(token, unicode.IsSpace) == -1 {
-					isParam = true
-				}
+			if i >= runEnd {
+				lookahead := trimEmptyListElements(header[i+1:])
+				runEnd = len(header) - len(lookahead)
+				runIsParam = startsWithAuthParam(lookahead)
 			}
 
-			if !isParam {
+			if !runIsParam {
 				// The part after the comma does not look like a parameter,
 				// so this comma separates challenges.
 				challenges = append(challenges, header[start:i])
@@ -224,6 +222,26 @@ func splitChallenges(header string) ([]string, error) {
 	return challenges, nil
 }
 
+// startsWithAuthParam reports whether s begins with a token followed by '='.
+// Whitespace between the token and '=' is allowed (BWS, RFC 9110 section 11.2).
+// It reads no further than that token, so its cost does not depend on the
+// length of the rest of the header.
+func startsWithAuthParam(s string) bool {
+	for i, r := range s {
+		switch {
+		case r == '=':
+			return i > 0
+		case r == ',':
+			return false
+		case r == ' ' || r == '\t':
+			return strings.HasPrefix(strings.TrimLeft(s[i:], " \t"), "=")
+		case unicode.IsSpace(r):
+			return false
+		}
+	}
+	return false
+}
+
 // parseSingleChallenge parses a string containing exactly one challenge.
 // challenge   = auth-scheme [ 1*SP ( token68 / #auth-param ) ]
 func parseSingleChallenge(s string) (Challenge, error) {

oauthex/resource_meta_test.go

@@ -9,7 +9,9 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"reflect"
+	"strings"
 	"testing"
+	"time"
 )
 
 func TestSplitChallenges(t *testing.T) {
@@ -68,6 +70,31 @@ func TestSplitChallenges(t *testing.T) {
 			input: "",
 			want:  []string{""},
 		},
+		{
+			name:  "challenges without params",
+			input: `Basic,Bearer`,
+			want:  []string{`Basic`, `Bearer`},
+		},
+		{
+			name:  "blank between token and equals",
+			input: "Bearer a=1, b \t=2",
+			want:  []string{"Bearer a=1, b \t=2"},
+		},
+		{
+			name:  "whitespace inside the name starts a new challenge",
+			input: `Basic realm="a", Bearer x y=1`,
+			want:  []string{`Basic realm="a"`, ` Bearer x y=1`},
+		},
+		{
+			name:  "non-blank whitespace before equals starts a new challenge",
+			input: "Basic, Bearer\u00a0=1",
+			want:  []string{"Basic", " Bearer\u00a0=1"},
+		},
+		{
+			name:  "comma inside the name starts a new challenge",
+			input: `Bearer x, a,b=c`,
+			want:  []string{`Bearer x`, ` a,b=c`},
+		},
 	}
 
 	for _, tt := range tests {
@@ -83,6 +110,28 @@ func TestSplitChallenges(t *testing.T) {
 	}
 }
 
+// The lookahead after a comma must not rescan the rest of the header, or a
+// long header from a server takes quadratic time that no context deadline can
+// interrupt. The input is kept small: the test only needs the old code to take
+// several times longer than the deadline, and it must not load the machine,
+// because other packages' tests run in parallel with it.
+func TestParseWWWAuthenticateLinearTime(t *testing.T) {
+	header := "Bearer " + strings.Repeat(",", 100_000)
+	done := make(chan error, 1)
+	go func() {
+		_, err := ParseWWWAuthenticate([]string{header})
+		done <- err
+	}()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatal(err)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatalf("ParseWWWAuthenticate did not finish within 2s on a %d byte header", len(header))
+	}
+}
+
 // A quoted string may end in an escaped backslash. The quote after it closes
 // the string, so the comma that follows still separates two challenges and
 // the Bearer challenge the auth handlers look for is not lost.