134 lines
4.2 KiB
Diff
134 lines
4.2 KiB
Diff
|
|
From ef41a4e2face45e580c5836eaebd51629fc23f15 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Damien Neil <dneil@google.com>
|
||
|
|
Date: Thu, 16 Mar 2023 14:18:04 -0700
|
||
|
|
Subject: [PATCH] [release-branch.go1.19] mime/multipart: avoid excessive copy
|
||
|
|
buffer allocations in ReadForm
|
||
|
|
|
||
|
|
When copying form data to disk with io.Copy,
|
||
|
|
allocate only one copy buffer and reuse it rather than
|
||
|
|
creating two buffers per file (one from io.multiReader.WriteTo,
|
||
|
|
and a second one from os.File.ReadFrom).
|
||
|
|
|
||
|
|
Thanks to Jakob Ackermann (@das7pad) for reporting this issue.
|
||
|
|
|
||
|
|
For CVE-2023-24536
|
||
|
|
For #59153
|
||
|
|
For #59269
|
||
|
|
|
||
|
|
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802453
|
||
|
|
Run-TryBot: Damien Neil <dneil@google.com>
|
||
|
|
Reviewed-by: Julie Qiu <julieqiu@google.com>
|
||
|
|
Reviewed-by: Roland Shoemaker <bracewell@google.com>
|
||
|
|
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1802395
|
||
|
|
Run-TryBot: Roland Shoemaker <bracewell@google.com>
|
||
|
|
Reviewed-by: Damien Neil <dneil@google.com>
|
||
|
|
Change-Id: Ie405470c92abffed3356913b37d813e982c96c8b
|
||
|
|
Reviewed-on: https://go-review.googlesource.com/c/go/+/481983
|
||
|
|
Run-TryBot: Michael Knyszek <mknyszek@google.com>
|
||
|
|
TryBot-Result: Gopher Robot <gobot@golang.org>
|
||
|
|
Auto-Submit: Michael Knyszek <mknyszek@google.com>
|
||
|
|
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
||
|
|
---
|
||
|
|
src/mime/multipart/formdata.go | 15 +++++++--
|
||
|
|
src/mime/multipart/formdata_test.go | 49 +++++++++++++++++++++++++++++
|
||
|
|
2 files changed, 61 insertions(+), 3 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go
|
||
|
|
index a7d4ca97f0..975dcb6b26 100644
|
||
|
|
--- a/src/mime/multipart/formdata.go
|
||
|
|
+++ b/src/mime/multipart/formdata.go
|
||
|
|
@@ -84,6 +84,7 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
|
||
|
|
maxMemoryBytes = math.MaxInt64
|
||
|
|
}
|
||
|
|
}
|
||
|
|
+ var copyBuf []byte
|
||
|
|
for {
|
||
|
|
p, err := r.nextPart(false, maxMemoryBytes)
|
||
|
|
if err == io.EOF {
|
||
|
|
@@ -147,14 +148,22 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) {
|
||
|
|
}
|
||
|
|
}
|
||
|
|
numDiskFiles++
|
||
|
|
- size, err := io.Copy(file, io.MultiReader(&b, p))
|
||
|
|
+ if _, err := file.Write(b.Bytes()); err != nil {
|
||
|
|
+ return nil, err
|
||
|
|
+ }
|
||
|
|
+ if copyBuf == nil {
|
||
|
|
+ copyBuf = make([]byte, 32*1024) // same buffer size as io.Copy uses
|
||
|
|
+ }
|
||
|
|
+ // os.File.ReadFrom will allocate its own copy buffer if we let io.Copy use it.
|
||
|
|
+ type writerOnly struct{ io.Writer }
|
||
|
|
+ remainingSize, err := io.CopyBuffer(writerOnly{file}, p, copyBuf)
|
||
|
|
if err != nil {
|
||
|
|
return nil, err
|
||
|
|
}
|
||
|
|
fh.tmpfile = file.Name()
|
||
|
|
- fh.Size = size
|
||
|
|
+ fh.Size = int64(b.Len()) + remainingSize
|
||
|
|
fh.tmpoff = fileOff
|
||
|
|
- fileOff += size
|
||
|
|
+ fileOff += fh.Size
|
||
|
|
if !combineFiles {
|
||
|
|
if err := file.Close(); err != nil {
|
||
|
|
return nil, err
|
||
|
|
diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go
|
||
|
|
index 5cded7170c..f5b56083b2 100644
|
||
|
|
--- a/src/mime/multipart/formdata_test.go
|
||
|
|
+++ b/src/mime/multipart/formdata_test.go
|
||
|
|
@@ -368,3 +368,52 @@ func testReadFormManyFiles(t *testing.T, distinct bool) {
|
||
|
|
t.Fatalf("temp dir contains %v files; want 0", len(names))
|
||
|
|
}
|
||
|
|
}
|
||
|
|
+
|
||
|
|
+func BenchmarkReadForm(b *testing.B) {
|
||
|
|
+ for _, test := range []struct {
|
||
|
|
+ name string
|
||
|
|
+ form func(fw *Writer, count int)
|
||
|
|
+ }{{
|
||
|
|
+ name: "fields",
|
||
|
|
+ form: func(fw *Writer, count int) {
|
||
|
|
+ for i := 0; i < count; i++ {
|
||
|
|
+ w, _ := fw.CreateFormField(fmt.Sprintf("field%v", i))
|
||
|
|
+ fmt.Fprintf(w, "value %v", i)
|
||
|
|
+ }
|
||
|
|
+ },
|
||
|
|
+ }, {
|
||
|
|
+ name: "files",
|
||
|
|
+ form: func(fw *Writer, count int) {
|
||
|
|
+ for i := 0; i < count; i++ {
|
||
|
|
+ w, _ := fw.CreateFormFile(fmt.Sprintf("field%v", i), fmt.Sprintf("file%v", i))
|
||
|
|
+ fmt.Fprintf(w, "value %v", i)
|
||
|
|
+ }
|
||
|
|
+ },
|
||
|
|
+ }} {
|
||
|
|
+ b.Run(test.name, func(b *testing.B) {
|
||
|
|
+ for _, maxMemory := range []int64{
|
||
|
|
+ 0,
|
||
|
|
+ 1 << 20,
|
||
|
|
+ } {
|
||
|
|
+ var buf bytes.Buffer
|
||
|
|
+ fw := NewWriter(&buf)
|
||
|
|
+ test.form(fw, 10)
|
||
|
|
+ if err := fw.Close(); err != nil {
|
||
|
|
+ b.Fatal(err)
|
||
|
|
+ }
|
||
|
|
+ b.Run(fmt.Sprintf("maxMemory=%v", maxMemory), func(b *testing.B) {
|
||
|
|
+ b.ReportAllocs()
|
||
|
|
+ for i := 0; i < b.N; i++ {
|
||
|
|
+ fr := NewReader(bytes.NewReader(buf.Bytes()), fw.Boundary())
|
||
|
|
+ form, err := fr.ReadForm(maxMemory)
|
||
|
|
+ if err != nil {
|
||
|
|
+ b.Fatal(err)
|
||
|
|
+ }
|
||
|
|
+ form.RemoveAll()
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
+ })
|
||
|
|
+ }
|
||
|
|
+ })
|
||
|
|
+ }
|
||
|
|
+}
|
||
|
|
--
|
||
|
|
2.37.1
|
||
|
|
|