Skip to content

Commit 72dccd0

Browse files
Fix extra whitespace parsing in HTTP request lines to prevent cache poisoning (#2061)
* Initial plan * Fix FastHTTP whitespace parsing issue - reject extra spaces in request lines Co-authored-by: erikdubbelboer <[email protected]> * Format header.go and header_test.go with gofmt Co-authored-by: erikdubbelboer <[email protected]> * Fix gofumpt formatting and mirror linting issues in header_test.go Co-authored-by: erikdubbelboer <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: erikdubbelboer <[email protected]>
1 parent 8c7d2bc commit 72dccd0

File tree

2 files changed

+89
-5
lines changed

2 files changed

+89
-5
lines changed

header.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2809,8 +2809,16 @@ func (h *RequestHeader) parseFirstLine(buf []byte) (int, error) {
28092809

28102810
b = b[n+1:]
28112811

2812-
// parse requestURI
2813-
n = bytes.LastIndexByte(b, ' ')
2812+
// Check for extra whitespace after method - only one space should separate method from URI
2813+
if len(b) > 0 && b[0] == ' ' {
2814+
if h.secureErrorLogMessage {
2815+
return 0, errors.New("extra whitespace in request line")
2816+
}
2817+
return 0, fmt.Errorf("extra whitespace in request line %q", buf)
2818+
}
2819+
2820+
// parse requestURI - RFC 9112 requires exactly one space between components
2821+
n = bytes.IndexByte(b, ' ')
28142822
if n < 0 {
28152823
return 0, fmt.Errorf("cannot find whitespace in the first line of request %q", buf)
28162824
} else if n == 0 {
@@ -2820,6 +2828,14 @@ func (h *RequestHeader) parseFirstLine(buf []byte) (int, error) {
28202828
return 0, fmt.Errorf("requestURI cannot be empty in %q", buf)
28212829
}
28222830

2831+
// Check for extra whitespace - only one space should separate URI from HTTP version
2832+
if n+1 < len(b) && b[n+1] == ' ' {
2833+
if h.secureErrorLogMessage {
2834+
return 0, errors.New("extra whitespace in request line")
2835+
}
2836+
return 0, fmt.Errorf("extra whitespace in request line %q", buf)
2837+
}
2838+
28232839
protoStr := b[n+1:]
28242840

28252841
// Follow RFCs 7230 and 9112 and require that HTTP versions match the following pattern: HTTP/[0-9]\.[0-9]

header_test.go

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2934,9 +2934,8 @@ func TestRequestHeaderReadSuccess(t *testing.T) {
29342934
testRequestHeaderReadSuccess(t, h, "\r\n\n\r\nGET /aaa HTTP/1.1\r\nHost: aaa.com\r\n\r\nsss",
29352935
-2, "/aaa", "aaa.com", "", "")
29362936

2937-
// request uri with spaces
2938-
testRequestHeaderReadSuccess(t, h, "GET /foo/ bar baz HTTP/1.1\r\nHost: aa.com\r\n\r\nxxx",
2939-
-2, "/foo/ bar baz", "aa.com", "", "")
2937+
// request uri with spaces - should be rejected per RFC 9112
2938+
testRequestHeaderReadError(t, h, "GET /foo/ bar baz HTTP/1.1\r\nHost: aa.com\r\n\r\nxxx")
29402939

29412940
// no host
29422941
testRequestHeaderReadSuccess(t, h, "GET /foo/bar HTTP/1.1\r\nFOObar: assdfd\r\n\r\naaa",
@@ -3404,3 +3403,72 @@ func TestAddVaryHeaderExistingAcceptEncoding(t *testing.T) {
34043403
t.Errorf("Vary occurred %d times", n)
34053404
}
34063405
}
3406+
3407+
func TestRequestHeaderExtraWhitespace(t *testing.T) {
3408+
var h RequestHeader
3409+
3410+
// Test cases that should fail due to extra whitespace
3411+
testCases := []string{
3412+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n", // Extra space after method
3413+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n", // Multiple spaces after method
3414+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n", // Extra space before HTTP version
3415+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n", // Multiple spaces before HTTP version
3416+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n", // Extra spaces in both places
3417+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n", // Multiple extra spaces in both places
3418+
}
3419+
3420+
for i, testCase := range testCases {
3421+
br := bufio.NewReader(bytes.NewBufferString(testCase))
3422+
err := h.Read(br)
3423+
if err == nil {
3424+
t.Errorf("Test case %d should have failed but didn't. Request: %q", i, testCase)
3425+
}
3426+
if !strings.Contains(err.Error(), "extra whitespace") {
3427+
t.Errorf("Test case %d should have failed with 'extra whitespace' error but got: %v", i, err)
3428+
}
3429+
}
3430+
}
3431+
3432+
func TestRequestHeaderValidWhitespace(t *testing.T) {
3433+
var h RequestHeader
3434+
3435+
// Test cases that should succeed (proper single space separation)
3436+
testCases := []struct {
3437+
request string
3438+
expectedMethod string
3439+
expectedURI string
3440+
}{
3441+
{
3442+
"GET /foo HTTP/1.1\r\nHost: example.com\r\n\r\n",
3443+
"GET",
3444+
"/foo",
3445+
},
3446+
{
3447+
"POST /api/v1/users HTTP/1.1\r\nHost: example.com\r\n\r\n",
3448+
"POST",
3449+
"/api/v1/users",
3450+
},
3451+
{
3452+
"PUT /resource HTTP/1.0\r\nHost: example.com\r\n\r\n",
3453+
"PUT",
3454+
"/resource",
3455+
},
3456+
}
3457+
3458+
for i, testCase := range testCases {
3459+
br := bufio.NewReader(bytes.NewBufferString(testCase.request))
3460+
err := h.Read(br)
3461+
if err != nil {
3462+
t.Errorf("Test case %d should have succeeded but failed with: %v. Request: %q", i, err, testCase.request)
3463+
continue
3464+
}
3465+
3466+
if string(h.Method()) != testCase.expectedMethod {
3467+
t.Errorf("Test case %d: expected method %q but got %q", i, testCase.expectedMethod, h.Method())
3468+
}
3469+
3470+
if string(h.RequestURI()) != testCase.expectedURI {
3471+
t.Errorf("Test case %d: expected URI %q but got %q", i, testCase.expectedURI, h.RequestURI())
3472+
}
3473+
}
3474+
}

0 commit comments

Comments
 (0)