Skip to content

Commit

Permalink
Don't allow \r in header names (#1789)
Browse files Browse the repository at this point in the history
* Don't allow \r in header names

From RFC 9112:

  A sender MUST NOT generate a bare CR (a CR character not immediately
  followed by LF) within any protocol elements other than the content.
  A recipient of such a bare CR MUST consider that element to be invalid
  or replace each bare CR with SP before processing the element or forwarding
  the message.

net/http seems to completely error on this, so let's do the same.

Fixes #1785

* Validate the full header field
  • Loading branch information
erikdubbelboer committed Jun 11, 2024
1 parent 9ffdf08 commit 7b273dc
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 9 deletions.
2 changes: 1 addition & 1 deletion bytesconv_table.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions bytesconv_table_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func main() {
return a
}()

tcharTable := func() [128]byte {
validHeaderFieldByteTable := func() [128]byte {
// Should match net/textproto's validHeaderFieldByte(c byte) bool
// Defined by RFC 9110 5.6.2
// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
Expand All @@ -125,7 +125,7 @@ func main() {
fmt.Fprintf(w, "const toUpperTable = %q\n", toUpperTable)
fmt.Fprintf(w, "const quotedArgShouldEscapeTable = %q\n", quotedArgShouldEscapeTable)
fmt.Fprintf(w, "const quotedPathShouldEscapeTable = %q\n", quotedPathShouldEscapeTable)
fmt.Fprintf(w, "const tcharTable = %q\n", tcharTable)
fmt.Fprintf(w, "const validHeaderFieldByteTable = %q\n", validHeaderFieldByteTable)

if err := os.WriteFile("bytesconv_table.go", w.Bytes(), 0o660); err != nil {
log.Fatal(err)
Expand Down
23 changes: 17 additions & 6 deletions header.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func (h *ResponseHeader) AddTrailerBytes(trailer []byte) error {
// validHeaderFieldByte returns true if c is a valid tchar as defined
// by section 5.6.2 of [RFC9110].
func validHeaderFieldByte(c byte) bool {
return c < 128 && tcharTable[c] == 1
return c < 128 && validHeaderFieldByteTable[c] == 1
}

// VisitHeaderParams calls f for each parameter in the given header bytes.
Expand Down Expand Up @@ -2947,8 +2947,17 @@ func (h *ResponseHeader) parseHeaders(buf []byte) (int, error) {
s.disableNormalizing = h.disableNormalizing
var err error
var kv *argsKV

outer:
for s.next() {
if len(s.key) > 0 {
for _, ch := range s.key {
if !validHeaderFieldByte(ch) {
err = fmt.Errorf("invalid header key %q", s.key)
continue outer
}
}

switch s.key[0] | 0x20 {
case 'c':
if caseInsensitiveCompare(s.key, strContentType) {
Expand Down Expand Up @@ -3035,13 +3044,15 @@ func (h *RequestHeader) parseHeaders(buf []byte) (int, error) {
s.b = buf
s.disableNormalizing = h.disableNormalizing
var err error

outer:
for s.next() {
if len(s.key) > 0 {
// Spaces between the header key and colon are not allowed.
// See RFC 7230, Section 3.2.4.
if bytes.IndexByte(s.key, ' ') != -1 || bytes.IndexByte(s.key, '\t') != -1 {
err = fmt.Errorf("invalid header key %q", s.key)
continue
for _, ch := range s.key {
if !validHeaderFieldByte(ch) {
err = fmt.Errorf("invalid header key %q", s.key)
continue outer
}
}

if h.disableSpecialHeader {
Expand Down
13 changes: 13 additions & 0 deletions header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,19 @@ func TestResponseHeaderMultiLinePanicked(t *testing.T) {
}
}

func TestRequestHeaderLooseBackslashR(t *testing.T) {
t.Parallel()

s := "GET / HTTP/1.1\r\n" +
"Host: go.dev\r\n" +
"\rFoo: bar\r\n" +
"\r\n"
header := new(RequestHeader)
if _, err := header.parse([]byte(s)); err == nil {
t.Fatal("expected error, got <nil>")
}
}

func TestResponseHeaderEmptyValueFromHeader(t *testing.T) {
t.Parallel()

Expand Down

0 comments on commit 7b273dc

Please sign in to comment.