From 0f53453b32054b681dac6e8b025c85a785396102 Mon Sep 17 00:00:00 2001 From: dqu123 Date: Sat, 10 Oct 2020 16:25:07 -0400 Subject: net/http: deep copy Request.TransferEncoding The existing implementation in Request.Clone() assigns the wrong pointer to r2.TransferEncoding. Fixes #41907 Change-Id: I7f220a41b1b46a55d1a1005e47c6dd69478cb025 Reviewed-on: https://go-review.googlesource.com/c/go/+/261258 Reviewed-by: Brad Fitzpatrick Reviewed-by: Emmanuel Odeke Trust: Emmanuel Odeke --- src/net/http/request_test.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'src/net/http/request_test.go') diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 42c16d00ea..461d66e05d 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -828,6 +828,27 @@ func TestWithContextDeepCopiesURL(t *testing.T) { } } +// Ensure that Request.Clone creates a deep copy of TransferEncoding. +// See issue 41907. +func TestRequestCloneTransferEncoding(t *testing.T) { + body := strings.NewReader("body") + req, _ := NewRequest("POST", "https://example.org/", body) + req.TransferEncoding = []string{ + "encoding1", + } + + clonedReq := req.Clone(context.Background()) + // modify original after deep copy + req.TransferEncoding[0] = "encoding2" + + if req.TransferEncoding[0] != "encoding2" { + t.Error("expected req.TransferEncoding to be changed") + } + if clonedReq.TransferEncoding[0] != "encoding1" { + t.Error("expected clonedReq.TransferEncoding to be unchanged") + } +} + func TestNoPanicOnRoundTripWithBasicAuth_h1(t *testing.T) { testNoPanicWithBasicAuth(t, h1Mode) } -- cgit v1.2.1 From 58eadc232e4fd2633761ffdeeaa922216beee74e Mon Sep 17 00:00:00 2001 From: avivklas Date: Tue, 15 Sep 2020 08:48:44 +0300 Subject: net/http: test that ParseMultipartForm returns an error for int overflow ParseMultipartForm has been changed to return an error if maxMemory parameter + 10MB causes int overflows. This adds a test for the new behaviour. For #40430 Change-Id: I4f66ce8a9382940182011d22a84ee52b1d1364cf Reviewed-on: https://go-review.googlesource.com/c/go/+/254977 Reviewed-by: Damien Neil Trust: Damien Neil Trust: Brad Fitzpatrick --- src/net/http/request_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) (limited to 'src/net/http/request_test.go') diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 461d66e05d..4f4f435814 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -13,6 +13,7 @@ import ( "fmt" "io" "io/ioutil" + "math" "mime/multipart" . "net/http" "net/http/httptest" @@ -245,6 +246,41 @@ func TestParseMultipartForm(t *testing.T) { } } +// Issue #40430: ParseMultipartForm should return error for int overflow +func TestMaxInt64ForMultipartFormMaxMemory(t *testing.T) { + cst := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { + if err := req.ParseMultipartForm(math.MaxInt64); err != nil { + Error(rw, err.Error(), StatusBadRequest) + return + } + })) + defer cst.Close() + fBuf := new(bytes.Buffer) + mw := multipart.NewWriter(fBuf) + mf, err := mw.CreateFormFile("file", "myfile.txt") + if err != nil { + t.Fatal(err) + } + if _, err := mf.Write(bytes.Repeat([]byte("abc"), 1<<10)); err != nil { + t.Fatal(err) + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req, err := NewRequest("POST", cst.URL, fBuf) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", mw.FormDataContentType()) + res, err := cst.Client().Do(req) + if err != nil { + t.Fatal(err) + } + if g, w := res.StatusCode, StatusBadRequest; g != w { + t.Fatalf("Status code mismatch: got %d, want %d", g, w) + } +} + func TestRedirect_h1(t *testing.T) { testRedirect(t, h1Mode) } func TestRedirect_h2(t *testing.T) { testRedirect(t, h2Mode) } func testRedirect(t *testing.T, h2 bool) { -- cgit v1.2.1 From 5647d01ab724a19793ac7002776b0dec03fa35f5 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 19 Oct 2020 18:03:51 +0000 Subject: Revert "net/http: test that ParseMultipartForm returns an error for int overflow" This reverts CL 254977. Reason for revert: introduced test failures on longtest builders. Change-Id: I75e868245f980189ad85dd4103d9178989e06ecf Reviewed-on: https://go-review.googlesource.com/c/go/+/263658 Trust: Bryan C. Mills Run-TryBot: Bryan C. Mills TryBot-Result: Go Bot Reviewed-by: Carlos Amedee Reviewed-by: Dmitri Shuralyov --- src/net/http/request_test.go | 36 ------------------------------------ 1 file changed, 36 deletions(-) (limited to 'src/net/http/request_test.go') diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 4f4f435814..461d66e05d 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -13,7 +13,6 @@ import ( "fmt" "io" "io/ioutil" - "math" "mime/multipart" . "net/http" "net/http/httptest" @@ -246,41 +245,6 @@ func TestParseMultipartForm(t *testing.T) { } } -// Issue #40430: ParseMultipartForm should return error for int overflow -func TestMaxInt64ForMultipartFormMaxMemory(t *testing.T) { - cst := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { - if err := req.ParseMultipartForm(math.MaxInt64); err != nil { - Error(rw, err.Error(), StatusBadRequest) - return - } - })) - defer cst.Close() - fBuf := new(bytes.Buffer) - mw := multipart.NewWriter(fBuf) - mf, err := mw.CreateFormFile("file", "myfile.txt") - if err != nil { - t.Fatal(err) - } - if _, err := mf.Write(bytes.Repeat([]byte("abc"), 1<<10)); err != nil { - t.Fatal(err) - } - if err := mw.Close(); err != nil { - t.Fatal(err) - } - req, err := NewRequest("POST", cst.URL, fBuf) - if err != nil { - t.Fatal(err) - } - req.Header.Set("Content-Type", mw.FormDataContentType()) - res, err := cst.Client().Do(req) - if err != nil { - t.Fatal(err) - } - if g, w := res.StatusCode, StatusBadRequest; g != w { - t.Fatalf("Status code mismatch: got %d, want %d", g, w) - } -} - func TestRedirect_h1(t *testing.T) { testRedirect(t, h1Mode) } func TestRedirect_h2(t *testing.T) { testRedirect(t, h2Mode) } func testRedirect(t *testing.T, h2 bool) { -- cgit v1.2.1 From 1b09d430678d4a6f73b2443463d11f75851aba8a Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 16 Oct 2020 00:49:02 -0400 Subject: all: update references to symbols moved from io/ioutil to io The old ioutil references are still valid, but update our code to reflect best practices and get used to the new locations. Code compiled with the bootstrap toolchain (cmd/asm, cmd/dist, cmd/compile, debug/elf) must remain Go 1.4-compatible and is excluded. Also excluded vendored code. For #41190. Change-Id: I6d86f2bf7bc37a9d904b6cee3fe0c7af6d94d5b1 Reviewed-on: https://go-review.googlesource.com/c/go/+/263142 Trust: Russ Cox Run-TryBot: Russ Cox TryBot-Result: Go Bot Reviewed-by: Emmanuel Odeke --- src/net/http/request_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'src/net/http/request_test.go') diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index 461d66e05d..b4ef472e71 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -103,7 +103,7 @@ func TestParseFormUnknownContentType(t *testing.T) { req := &Request{ Method: "POST", Header: test.contentType, - Body: ioutil.NopCloser(strings.NewReader("body")), + Body: io.NopCloser(strings.NewReader("body")), } err := req.ParseForm() switch { @@ -150,7 +150,7 @@ func TestMultipartReader(t *testing.T) { req := &Request{ Method: "POST", Header: Header{"Content-Type": {test.contentType}}, - Body: ioutil.NopCloser(new(bytes.Buffer)), + Body: io.NopCloser(new(bytes.Buffer)), } multipart, err := req.MultipartReader() if test.shouldError { @@ -187,7 +187,7 @@ binary data req := &Request{ Method: "POST", Header: Header{"Content-Type": {`multipart/form-data; boundary=xxx`}}, - Body: ioutil.NopCloser(strings.NewReader(postData)), + Body: io.NopCloser(strings.NewReader(postData)), } initialFormItems := map[string]string{ @@ -231,7 +231,7 @@ func TestParseMultipartForm(t *testing.T) { req := &Request{ Method: "POST", Header: Header{"Content-Type": {`multipart/form-data; boundary="foo123"`}}, - Body: ioutil.NopCloser(new(bytes.Buffer)), + Body: io.NopCloser(new(bytes.Buffer)), } err := req.ParseMultipartForm(25) if err == nil { @@ -756,10 +756,10 @@ func (dr delayedEOFReader) Read(p []byte) (n int, err error) { } func TestIssue10884_MaxBytesEOF(t *testing.T) { - dst := ioutil.Discard + dst := io.Discard _, err := io.Copy(dst, MaxBytesReader( responseWriterJustWriter{dst}, - ioutil.NopCloser(delayedEOFReader{strings.NewReader("12345")}), + io.NopCloser(delayedEOFReader{strings.NewReader("12345")}), 5)) if err != nil { t.Fatal(err) @@ -799,7 +799,7 @@ func TestMaxBytesReaderStickyError(t *testing.T) { 2: {101, 100}, } for i, tt := range tests { - rc := MaxBytesReader(nil, ioutil.NopCloser(bytes.NewReader(make([]byte, tt.readable))), tt.limit) + rc := MaxBytesReader(nil, io.NopCloser(bytes.NewReader(make([]byte, tt.readable))), tt.limit) if err := isSticky(rc); err != nil { t.Errorf("%d. error: %v", i, err) } @@ -900,7 +900,7 @@ func TestNewRequestGetBody(t *testing.T) { t.Errorf("test[%d]: GetBody = nil", i) continue } - slurp1, err := ioutil.ReadAll(req.Body) + slurp1, err := io.ReadAll(req.Body) if err != nil { t.Errorf("test[%d]: ReadAll(Body) = %v", i, err) } @@ -908,7 +908,7 @@ func TestNewRequestGetBody(t *testing.T) { if err != nil { t.Errorf("test[%d]: GetBody = %v", i, err) } - slurp2, err := ioutil.ReadAll(newBody) + slurp2, err := io.ReadAll(newBody) if err != nil { t.Errorf("test[%d]: ReadAll(GetBody()) = %v", i, err) } @@ -1145,7 +1145,7 @@ func benchmarkFileAndServer(b *testing.B, n int64) { func runFileAndServerBenchmarks(b *testing.B, tlsOption bool, f *os.File, n int64) { handler := HandlerFunc(func(rw ResponseWriter, req *Request) { defer req.Body.Close() - nc, err := io.Copy(ioutil.Discard, req.Body) + nc, err := io.Copy(io.Discard, req.Body) if err != nil { panic(err) } @@ -1172,7 +1172,7 @@ func runFileAndServerBenchmarks(b *testing.B, tlsOption bool, f *os.File, n int6 } b.StartTimer() - req, err := NewRequest("PUT", cst.URL, ioutil.NopCloser(f)) + req, err := NewRequest("PUT", cst.URL, io.NopCloser(f)) if err != nil { b.Fatal(err) } -- cgit v1.2.1 From 4e5a313524da62600eb59dbf98624cfe946456f8 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Tue, 20 Oct 2020 04:11:12 -0700 Subject: net/http: test that ParseMultipartForm catches overflows Tests that if the combination of: * HTTP multipart file payload size * ParseMultipartForm's maxMemory parameter * the internal leeway buffer size of 10MiB overflows, then we'll report an overflow instead of silently passing. Reapplies and fixes CL 254977, which was reverted in CL 263658. The prior test lacked a res.Body.Close(), so fixed that and added a leaked Transport check to verify correctness. Updates 40430. Change-Id: I3c0f7ef43d621f6eb00f07755f04f9f36c51f98f Reviewed-on: https://go-review.googlesource.com/c/go/+/263817 Run-TryBot: Emmanuel Odeke TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills Trust: Damien Neil --- src/net/http/request_test.go | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) (limited to 'src/net/http/request_test.go') diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index b4ef472e71..19526b9ad7 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -13,6 +13,7 @@ import ( "fmt" "io" "io/ioutil" + "math" "mime/multipart" . "net/http" "net/http/httptest" @@ -245,6 +246,50 @@ func TestParseMultipartForm(t *testing.T) { } } +// Issue #40430: Test that if maxMemory for ParseMultipartForm when combined with +// the payload size and the internal leeway buffer size of 10MiB overflows, that we +// correctly return an error. +func TestMaxInt64ForMultipartFormMaxMemoryOverflow(t *testing.T) { + defer afterTest(t) + + payloadSize := 1 << 10 + cst := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { + // The combination of: + // MaxInt64 + payloadSize + (internal spare of 10MiB) + // triggers the overflow. See issue https://golang.org/issue/40430/ + if err := req.ParseMultipartForm(math.MaxInt64); err != nil { + Error(rw, err.Error(), StatusBadRequest) + return + } + })) + defer cst.Close() + fBuf := new(bytes.Buffer) + mw := multipart.NewWriter(fBuf) + mf, err := mw.CreateFormFile("file", "myfile.txt") + if err != nil { + t.Fatal(err) + } + if _, err := mf.Write(bytes.Repeat([]byte("abc"), payloadSize)); err != nil { + t.Fatal(err) + } + if err := mw.Close(); err != nil { + t.Fatal(err) + } + req, err := NewRequest("POST", cst.URL, fBuf) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", mw.FormDataContentType()) + res, err := cst.Client().Do(req) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + if g, w := res.StatusCode, StatusBadRequest; g != w { + t.Fatalf("Status code mismatch: got %d, want %d", g, w) + } +} + func TestRedirect_h1(t *testing.T) { testRedirect(t, h1Mode) } func TestRedirect_h2(t *testing.T) { testRedirect(t, h2Mode) } func testRedirect(t *testing.T, h2 bool) { -- cgit v1.2.1