From 27695fa0b1232ef4518ae0cab807d159c294358f Mon Sep 17 00:00:00 2001
From: Demi Marie Obenour <demi@invisiblethingslab.com>
Date: Sun, 13 Apr 2025 03:02:38 -0400
Subject: [PATCH] http/httpguts: reject leading and trailing spaces in field
 values

RFC9113 is extremely clear about this:

> A field value MUST NOT start or end with an ASCII whitespace character
> (ASCII SP or HTAB, 0x20 or 0x09).

RFC9114 defers to RFC9110, which in turn provides a grammer that does
not permit leading and/or trailing whitespace either.
---
 http/httpguts/httplex.go      | 63 +++++++++++++----------------------
 http/httpguts/httplex_test.go | 39 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 39 deletions(-)

diff --git a/http/httpguts/httplex.go b/http/httpguts/httplex.go
index 9b4de94019..079ffc5960 100644
--- a/http/httpguts/httplex.go
+++ b/http/httpguts/httplex.go
@@ -262,48 +262,33 @@ var validHostByte = [256]bool{
 	'~':  true, // unreserved
 }
 
+// validFieldValueChar reports whether v is an RFC9110 field-vchar, SP, or HTAB.
+func validFieldValueChar(v uint8) bool {
+	if v < ' ' {
+		return v == '\t'
+	} else {
+		return v != 0x7F
+	}
+}
+
 // ValidHeaderFieldValue reports whether v is a valid "field-value" according to
-// http://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2 :
-//
-//	message-header = field-name ":" [ field-value ]
-//	field-value    = *( field-content | LWS )
-//	field-content  = <the OCTETs making up the field-value
-//	                 and consisting of either *TEXT or combinations
-//	                 of token, separators, and quoted-string>
-//
-// http://www.w3.org/Protocols/rfc2616/rfc2616-sec2.html#sec2.2 :
-//
-//	TEXT           = <any OCTET except CTLs,
-//	                  but including LWS>
-//	LWS            = [CRLF] 1*( SP | HT )
-//	CTL            = <any US-ASCII control character
-//	                 (octets 0 - 31) and DEL (127)>
+// <https://rfc-editor.org/rfc/rfc9110#name-field-values>:
 //
-// RFC 7230 says:
-//
-//	field-value    = *( field-content / obs-fold )
-//	obj-fold       =  N/A to http2, and deprecated
-//	field-content  = field-vchar [ 1*( SP / HTAB ) field-vchar ]
-//	field-vchar    = VCHAR / obs-text
-//	obs-text       = %x80-FF
-//	VCHAR          = "any visible [USASCII] character"
-//
-// http2 further says: "Similarly, HTTP/2 allows header field values
-// that are not valid. While most of the values that can be encoded
-// will not alter header field parsing, carriage return (CR, ASCII
-// 0xd), line feed (LF, ASCII 0xa), and the zero character (NUL, ASCII
-// 0x0) might be exploited by an attacker if they are translated
-// verbatim. Any request or response that contains a character not
-// permitted in a header field value MUST be treated as malformed
-// (Section 8.1.2.6). Valid characters are defined by the
-// field-content ABNF rule in Section 3.2 of [RFC7230]."
-//
-// This function does not (yet?) properly handle the rejection of
-// strings that begin or end with SP or HTAB.
+//        field-value    = *field-content
+//        field-content  = field-vchar
+//                         [ 1*( SP / HTAB / field-vchar ) field-vchar ]
+//        field-vchar    = VCHAR / obs-text
+//        obs-text       = %x80-FF
 func ValidHeaderFieldValue(v string) bool {
-	for i := 0; i < len(v); i++ {
-		b := v[i]
-		if isCTL(b) && !isLWS(b) {
+	l := len(v)
+	if l == 0 {
+		return true
+	}
+	if v[0] <= ' ' || v[l - 1] <= ' ' {
+		return false
+	}
+	for i := 0; i < l; i++ {
+		if !validFieldValueChar(v[i]) {
 			return false
 		}
 	}
diff --git a/http/httpguts/httplex_test.go b/http/httpguts/httplex_test.go
index 791440b1a7..4fbaabc2a9 100644
--- a/http/httpguts/httplex_test.go
+++ b/http/httpguts/httplex_test.go
@@ -129,6 +129,45 @@ func TestValidHeaderFieldName(t *testing.T) {
 	}
 }
 
+func TestValidHeaderFieldValue(t *testing.T) {
+	tests := []struct {
+		in   string
+		want bool
+	}{
+		{"", true},
+		{" junk", false},
+		{"\tjunk", false},
+		{"junk\t", false},
+		{"junk ", false},
+		{" ", false},
+		{"\t", false},
+	}
+	for i := byte(0); true; i++ {
+		bad := i < ' '
+		if i == 0x7f {
+			bad = true
+		}
+		if i == '\t' {
+			bad = false
+		}
+		tests = append(tests,
+			struct {
+				in   string
+				want bool
+			}{string([]byte{'a', i, 'b'}), !bad})
+		if i == 255 {
+			break
+		}
+	}
+
+	for _, tt := range tests {
+		got := ValidHeaderFieldValue(tt.in)
+		if tt.want != got {
+			t.Errorf("ValidHeaderFieldValue(%q) = %t; want %t", tt.in, got, tt.want)
+		}
+	}
+}
+
 func BenchmarkValidHeaderFieldName(b *testing.B) {
 	names := []string{
 		"",