diff --git a/oauthex/resource_meta.go b/oauthex/resource_meta.go index 880182bc2..2869c82f9 100644 --- a/oauthex/resource_meta.go +++ b/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) { diff --git a/oauthex/resource_meta_test.go b/oauthex/resource_meta_test.go index c262bffc7..c6678006c 100644 --- a/oauthex/resource_meta_test.go +++ b/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.