net/http: disallow empty Content-Length header

The Content-Length must be a valid numeric value, empty values should not be accepted.

See: https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length

Fixes #61679

Change-Id: Icbcd933087fe5e50199b62ff34c58bf92a09d3d4
GitHub-Last-Rev: 932e46b55b54d5f2050453bcaa50e9476c8559fd
GitHub-Pull-Request: golang/go#61865
Reviewed-on: https://go-review.googlesource.com/c/go/+/517336
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/doc/godebug.md b/doc/godebug.md
index d26555503e0522072b8ed75d7e63801547794c33..35aac00d76edf69f339055a65f099f14d65a0822 100644 (file)
--- a/doc/godebug.md
+++ b/doc/godebug.md
@@ -134,6 +134,10 @@ The default is tlsmaxrsasize=8192, limiting RSA to 8192-bit keys. To avoid
 denial of service attacks, this setting and default was backported to Go
 1.19.13, Go 1.20.8, and Go 1.21.1.
 
+Go 1.22 made it an error for a request or response read by a net/http
+client or server to have an empty Content-Length header.
+This behavior is controlled by the `httplaxcontentlength` setting.
+
 ### Go 1.21
 
 Go 1.21 made it a run-time error to call `panic` with a nil interface value,

diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go
index b1711d9ef21f8526dfe671ec6809015b557c4b4d..cc169e6661e24fff09dba385ab3a0662b36dc2dc 100644 (file)
--- a/src/internal/godebugs/table.go
+++ b/src/internal/godebugs/table.go
@@ -32,6 +32,7 @@ var All = []Info{
        {Name: "http2client", Package: "net/http"},
        {Name: "http2debug", Package: "net/http", Opaque: true},
        {Name: "http2server", Package: "net/http"},
+       {Name: "httplaxcontentlength", Package: "net/http", Changed: 22, Old: "1"},
        {Name: "installgoroot", Package: "go/build"},
        {Name: "jstmpllitinterp", Package: "html/template"},
        //{Name: "multipartfiles", Package: "mime/multipart"},

diff --git a/src/net/http/response_test.go b/src/net/http/response_test.go
index 19fb48f23ce194a8e420a0d0c06540a52f9209eb..ddd318084db4733b4fcb21737e825f6656c7bb3f 100644 (file)
--- a/src/net/http/response_test.go
+++ b/src/net/http/response_test.go
@@ -883,6 +883,7 @@ func TestReadResponseErrors(t *testing.T) {
        }
 
        errMultiCL := "message cannot contain multiple Content-Length headers"
+       errEmptyCL := "invalid empty Content-Length"
 
        tests := []testCase{
                {"", "", io.ErrUnexpectedEOF},
@@ -918,7 +919,7 @@ func TestReadResponseErrors(t *testing.T) {
                contentLength("200 OK", "Content-Length: 7\r\nContent-Length: 7\r\n\r\nGophers\r\n", nil),
                contentLength("201 OK", "Content-Length: 0\r\nContent-Length: 7\r\n\r\nGophers\r\n", errMultiCL),
                contentLength("300 OK", "Content-Length: 0\r\nContent-Length: 0 \r\n\r\nGophers\r\n", nil),
-               contentLength("200 OK", "Content-Length:\r\nContent-Length:\r\n\r\nGophers\r\n", nil),
+               contentLength("200 OK", "Content-Length:\r\nContent-Length:\r\n\r\nGophers\r\n", errEmptyCL),
                contentLength("206 OK", "Content-Length:\r\nContent-Length: 0 \r\nConnection: close\r\n\r\nGophers\r\n", errMultiCL),
 
                // multiple content-length headers for 204 and 304 should still be checked

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index d6f26a709c3c0e132c4b806fc6b7441b94377c36..b24998174f6ecc9dba8a91d0115d9cb8d6ad81fc 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -9,6 +9,7 @@ import (
        "bytes"
        "errors"
        "fmt"
+       "internal/godebug"
        "io"
        "net/http/httptrace"
        "net/http/internal"
@@ -527,7 +528,7 @@ func readTransfer(msg any, r *bufio.Reader) (err error) {
                return err
        }
        if isResponse && t.RequestMethod == "HEAD" {
-               if n, err := parseContentLength(t.Header.get("Content-Length")); err != nil {
+               if n, err := parseContentLength(t.Header["Content-Length"]); err != nil {
                        return err
                } else {
                        t.ContentLength = n
@@ -707,18 +708,15 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header,
                return -1, nil
        }
 
-       // Logic based on Content-Length
-       var cl string
-       if len(contentLens) == 1 {
-               cl = textproto.TrimString(contentLens[0])
-       }
-       if cl != "" {
-               n, err := parseContentLength(cl)
+       if len(contentLens) > 0 {
+               // Logic based on Content-Length
+               n, err := parseContentLength(contentLens)
                if err != nil {
                        return -1, err
                }
                return n, nil
        }
+
        header.Del("Content-Length")
 
        if isRequest {
@@ -1038,19 +1036,31 @@ func (bl bodyLocked) Read(p []byte) (n int, err error) {
        return bl.b.readLocked(p)
 }
 
-// parseContentLength trims whitespace from s and returns -1 if no value
-// is set, or the value if it's >= 0.
-func parseContentLength(cl string) (int64, error) {
-       cl = textproto.TrimString(cl)
-       if cl == "" {
+var laxContentLength = godebug.New("httplaxcontentlength")
+
+// parseContentLength checks that the header is valid and then trims
+// whitespace. It returns -1 if no value is set otherwise the value
+// if it's >= 0.
+func parseContentLength(clHeaders []string) (int64, error) {
+       if len(clHeaders) == 0 {
                return -1, nil
        }
+       cl := textproto.TrimString(clHeaders[0])
+
+       // The Content-Length must be a valid numeric value.
+       // See: https://datatracker.ietf.org/doc/html/rfc2616/#section-14.13
+       if cl == "" {
+               if laxContentLength.Value() == "1" {
+                       laxContentLength.IncNonDefault()
+                       return -1, nil
+               }
+               return 0, badStringError("invalid empty Content-Length", cl)
+       }
        n, err := strconv.ParseUint(cl, 10, 63)
        if err != nil {
                return 0, badStringError("bad Content-Length", cl)
        }
        return int64(n), nil
-
 }
 
 // finishAsyncByteRead finishes reading the 1-byte sniff

diff --git a/src/net/http/transfer_test.go b/src/net/http/transfer_test.go
index 5e0df896d80c818a8d5d7ecac23baaf41b143b09..20cc7b5d50b7a1e4dc1d1af588190ee44a7af0d3 100644 (file)
--- a/src/net/http/transfer_test.go
+++ b/src/net/http/transfer_test.go
@@ -332,6 +332,10 @@ func TestParseContentLength(t *testing.T) {
                cl      string
                wantErr error
        }{
+               {
+                       cl:      "",
+                       wantErr: badStringError("invalid empty Content-Length", ""),
+               },
                {
                        cl:      "3",
                        wantErr: nil,
@@ -356,7 +360,7 @@ func TestParseContentLength(t *testing.T) {
        }
 
        for _, tt := range tests {
-               if _, gotErr := parseContentLength(tt.cl); !reflect.DeepEqual(gotErr, tt.wantErr) {
+               if _, gotErr := parseContentLength([]string{tt.cl}); !reflect.DeepEqual(gotErr, tt.wantErr) {
                        t.Errorf("%q:\n\tgot=%v\n\twant=%v", tt.cl, gotErr, tt.wantErr)
                }
        }

diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go
index 55d1f65f42d11ce0090b4c50383d51d82fb3ccfc..bf7c96f8b5972cd9336e6edc0f0729078198446a 100644 (file)
--- a/src/runtime/metrics/doc.go
+++ b/src/runtime/metrics/doc.go
@@ -254,6 +254,11 @@ Below is the full list of supported metrics, ordered lexicographically.
                The number of non-default behaviors executed by the net/http
                package due to a non-default GODEBUG=http2server=... setting.
 
+       /godebug/non-default-behavior/httplaxcontentlength:events
+               The number of non-default behaviors executed by the net/http
+               package due to a non-default GODEBUG=httplaxcontentlength=...
+               setting.
+
        /godebug/non-default-behavior/installgoroot:events
                The number of non-default behaviors executed by the go/build
                package due to a non-default GODEBUG=installgoroot=... setting.