net/http: reject HTTP/1.1 Content-Length with sign in response

Enforces section 14.13 of RFC 2616 so that Content-Length header
values with a sign such as "+5" will be rejected.

Updates #39017

Change-Id: Icce9f00d03c8475fe704b33f9bed9089ff8802f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/234817
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>

diff --git a/src/net/http/httptest/recorder.go b/src/net/http/httptest/recorder.go
index 13697454cb08f67c46bed9d918d7930f423b9d5f..66e67e78b3031d418297d9b44a615bbc7b8594f7 100644 (file)
--- a/src/net/http/httptest/recorder.go
+++ b/src/net/http/httptest/recorder.go
@@ -226,9 +226,9 @@ func parseContentLength(cl string) int64 {
        if cl == "" {
                return -1
        }
-       n, err := strconv.ParseInt(cl, 10, 64)
+       n, err := strconv.ParseUint(cl, 10, 63)
        if err != nil {
                return -1
        }
-       return n
+       return int64(n)
 }

diff --git a/src/net/http/httptest/recorder_test.go b/src/net/http/httptest/recorder_test.go
index 0986554aa88e2e1eee685f3913a8f8bdef2b3f0e..e9534894b6c3942be5d569e5e94d3f1beaa087dc 100644 (file)
--- a/src/net/http/httptest/recorder_test.go
+++ b/src/net/http/httptest/recorder_test.go
@@ -310,3 +310,39 @@ func TestRecorder(t *testing.T) {
                })
        }
 }
+
+// issue 39017 - disallow Content-Length values such as "+3"
+func TestParseContentLength(t *testing.T) {
+       tests := []struct {
+               cl   string
+               want int64
+       }{
+               {
+                       cl:   "3",
+                       want: 3,
+               },
+               {
+                       cl:   "+3",
+                       want: -1,
+               },
+               {
+                       cl:   "-3",
+                       want: -1,
+               },
+               {
+                       // max int64, for safe conversion before returning
+                       cl:   "9223372036854775807",
+                       want: 9223372036854775807,
+               },
+               {
+                       cl:   "9223372036854775808",
+                       want: -1,
+               },
+       }
+
+       for _, tt := range tests {
+               if got := parseContentLength(tt.cl); got != tt.want {
+                       t.Errorf("%q:\n\tgot=%d\n\twant=%d", tt.cl, got, tt.want)
+               }
+       }
+}

diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go
index 9019afb61d442216dc5e1c6b1fa734dfe1108e9e..50d434b1fb096079a1594db94a05d9710b43270a 100644 (file)
--- a/src/net/http/transfer.go
+++ b/src/net/http/transfer.go
@@ -1039,11 +1039,11 @@ func parseContentLength(cl string) (int64, error) {
        if cl == "" {
                return -1, nil
        }
-       n, err := strconv.ParseInt(cl, 10, 64)
-       if err != nil || n < 0 {
+       n, err := strconv.ParseUint(cl, 10, 63)
+       if err != nil {
                return 0, badStringError("bad Content-Length", cl)
        }
-       return n, nil
+       return int64(n), nil
 
 }
 

diff --git a/src/net/http/transfer_test.go b/src/net/http/transfer_test.go
index e27d34dd7812a3bf5adb0360f373baacd489c70a..185225fa931359fa894dfc476e35d0218fda6172 100644 (file)
--- a/src/net/http/transfer_test.go
+++ b/src/net/http/transfer_test.go
@@ -326,3 +326,39 @@ func TestParseTransferEncoding(t *testing.T) {
                }
        }
 }
+
+// issue 39017 - disallow Content-Length values such as "+3"
+func TestParseContentLength(t *testing.T) {
+       tests := []struct {
+               cl      string
+               wantErr error
+       }{
+               {
+                       cl:      "3",
+                       wantErr: nil,
+               },
+               {
+                       cl:      "+3",
+                       wantErr: badStringError("bad Content-Length", "+3"),
+               },
+               {
+                       cl:      "-3",
+                       wantErr: badStringError("bad Content-Length", "-3"),
+               },
+               {
+                       // max int64, for safe conversion before returning
+                       cl:      "9223372036854775807",
+                       wantErr: nil,
+               },
+               {
+                       cl:      "9223372036854775808",
+                       wantErr: badStringError("bad Content-Length", "9223372036854775808"),
+               },
+       }
+
+       for _, tt := range tests {
+               if _, gotErr := parseContentLength(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/net/http/transport_test.go b/src/net/http/transport_test.go
index 5ccb3d14abd2d0145788e41caebfaae64f65112d..99056a42d90d252b67d0a2fbc7de2f9aa63b9a8f 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6222,3 +6222,22 @@ func TestIssue32441(t *testing.T) {
                t.Error(err)
        }
 }
+
+// Issue 39017. Ensure that HTTP/1 transports reject Content-Length headers
+// that contain a sign (eg. "+3"), per RFC 2616, Section 14.13.
+func TestTransportRejectsSignInContentLength(t *testing.T) {
+       cst := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               w.Header().Set("Content-Length", "+3")
+               w.Write([]byte("abc"))
+       }))
+       defer cst.Close()
+
+       c := cst.Client()
+       res, err := c.Get(cst.URL)
+       if err == nil || res != nil {
+               t.Fatal("Expected a non-nil error and a nil http.Response")
+       }
+       if got, want := err.Error(), `bad Content-Length "+3"`; !strings.Contains(got, want) {
+               t.Fatalf("Error mismatch\nGot: %q\nWanted substring: %q", got, want)
+       }
+}