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.
 
 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,
 ### 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: "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"},
        {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"
        }
 
        errMultiCL := "message cannot contain multiple Content-Length headers"

+       errEmptyCL := "invalid empty Content-Length"

 
        tests := []testCase{
                {"", "", io.ErrUnexpectedEOF},
 
        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: 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
                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"
        "bytes"
        "errors"
        "fmt"

+       "internal/godebug"

        "io"
        "net/http/httptrace"
        "net/http/internal"
        "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" {
                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
                        return err
                } else {
                        t.ContentLength = n


@@ -707,18 +708,15 @@ func fixLength(isResponse bool, status int, requestMethod string, header Header,
                return -1, nil
        }
 
                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
        }
                if err != nil {
                        return -1, err
                }
                return n, nil
        }

+

        header.Del("Content-Length")
 
        if isRequest {
        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)
 }
 
        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
        }
                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
        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
 }
 
 // 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      string
                wantErr error
        }{

+               {
+                       cl:      "",
+                       wantErr: badStringError("invalid empty Content-Length", ""),
+               },

                {
                        cl:      "3",
                        wantErr: nil,
                {
                        cl:      "3",
                        wantErr: nil,


@@ -356,7 +360,7 @@ func TestParseContentLength(t *testing.T) {
        }
 
        for _, tt := range tests {
        }
 
        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)
                }
        }
                        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.
 
                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.
        /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.