diff options
| -rw-r--r-- | src/mime/multipart/formdata.go | 9 | ||||
| -rw-r--r-- | src/mime/multipart/formdata_test.go | 55 | ||||
| -rw-r--r-- | src/net/textproto/reader.go | 8 |
3 files changed, 37 insertions, 35 deletions
diff --git a/src/mime/multipart/formdata.go b/src/mime/multipart/formdata.go index 975dcb6b26..3f6ff697ca 100644 --- a/src/mime/multipart/formdata.go +++ b/src/mime/multipart/formdata.go @@ -103,8 +103,9 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) { // Multiple values for the same key (one map entry, longer slice) are cheaper // than the same number of values for different keys (many map entries), but // using a consistent per-value cost for overhead is simpler. + const mapEntryOverhead = 200 maxMemoryBytes -= int64(len(name)) - maxMemoryBytes -= 100 // map overhead + maxMemoryBytes -= mapEntryOverhead if maxMemoryBytes < 0 { // We can't actually take this path, since nextPart would already have // rejected the MIME headers for being too large. Check anyway. @@ -128,7 +129,10 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) { } // file, store in memory or on disk + const fileHeaderSize = 100 maxMemoryBytes -= mimeHeaderSize(p.Header) + maxMemoryBytes -= mapEntryOverhead + maxMemoryBytes -= fileHeaderSize if maxMemoryBytes < 0 { return nil, ErrMessageTooLarge } @@ -183,9 +187,10 @@ func (r *Reader) readForm(maxMemory int64) (_ *Form, err error) { } func mimeHeaderSize(h textproto.MIMEHeader) (size int64) { + size = 400 for k, vs := range h { size += int64(len(k)) - size += 100 // map entry overhead + size += 200 // map entry overhead for _, v := range vs { size += int64(len(v)) } diff --git a/src/mime/multipart/formdata_test.go b/src/mime/multipart/formdata_test.go index f5b56083b2..8ed26e0c34 100644 --- a/src/mime/multipart/formdata_test.go +++ b/src/mime/multipart/formdata_test.go @@ -192,10 +192,10 @@ func (r *failOnReadAfterErrorReader) Read(p []byte) (n int, err error) { // TestReadForm_NonFileMaxMemory asserts that the ReadForm maxMemory limit is applied // while processing non-file form data as well as file form data. func TestReadForm_NonFileMaxMemory(t *testing.T) { - n := 10<<20 + 25 if testing.Short() { - n = 10<<10 + 25 + t.Skip("skipping in -short mode") } + n := 10 << 20 largeTextValue := strings.Repeat("1", n) message := `--MyBoundary Content-Disposition: form-data; name="largetext" @@ -203,38 +203,29 @@ Content-Disposition: form-data; name="largetext" ` + largeTextValue + ` --MyBoundary-- ` - testBody := strings.ReplaceAll(message, "\n", "\r\n") - testCases := []struct { - name string - maxMemory int64 - err error - }{ - {"smaller", 50 + int64(len("largetext")) + 100, nil}, - {"exact-fit", 25 + int64(len("largetext")) + 100, nil}, - {"too-large", 0, ErrMessageTooLarge}, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - if tc.maxMemory == 0 && testing.Short() { - t.Skip("skipping in -short mode") - } - b := strings.NewReader(testBody) - r := NewReader(b, boundary) - f, err := r.ReadForm(tc.maxMemory) - if err == nil { - defer f.RemoveAll() - } - if tc.err != err { - t.Fatalf("ReadForm error - got: %v; expected: %v", err, tc.err) - } - if err == nil { - if g := f.Value["largetext"][0]; g != largeTextValue { - t.Errorf("largetext mismatch: got size: %v, expected size: %v", len(g), len(largeTextValue)) - } - } - }) + // Try parsing the form with increasing maxMemory values. + // Changes in how we account for non-file form data may cause the exact point + // where we change from rejecting the form as too large to accepting it to vary, + // but we should see both successes and failures. + const failWhenMaxMemoryLessThan = 128 + for maxMemory := int64(0); maxMemory < failWhenMaxMemoryLessThan*2; maxMemory += 16 { + b := strings.NewReader(testBody) + r := NewReader(b, boundary) + f, err := r.ReadForm(maxMemory) + if err != nil { + continue + } + if g := f.Value["largetext"][0]; g != largeTextValue { + t.Errorf("largetext mismatch: got size: %v, expected size: %v", len(g), len(largeTextValue)) + } + f.RemoveAll() + if maxMemory < failWhenMaxMemoryLessThan { + t.Errorf("ReadForm(%v): no error, expect to hit memory limit when maxMemory < %v", maxMemory, failWhenMaxMemoryLessThan) + } + return } + t.Errorf("ReadForm(x) failed for x < 1024, expect success") } // TestReadForm_MetadataTooLarge verifies that we account for the size of field names, diff --git a/src/net/textproto/reader.go b/src/net/textproto/reader.go index 9a21777df8..c1284fde25 100644 --- a/src/net/textproto/reader.go +++ b/src/net/textproto/reader.go @@ -503,6 +503,12 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) { m := make(MIMEHeader, hint) + // Account for 400 bytes of overhead for the MIMEHeader, plus 200 bytes per entry. + // Benchmarking map creation as of go1.20, a one-entry MIMEHeader is 416 bytes and large + // MIMEHeaders average about 200 bytes per entry. + lim -= 400 + const mapEntryOverhead = 200 + // The first line cannot start with a leading space. if buf, err := r.R.Peek(1); err == nil && (buf[0] == ' ' || buf[0] == '\t') { line, err := r.readLineSlice() @@ -538,7 +544,7 @@ func readMIMEHeader(r *Reader, lim int64) (MIMEHeader, error) { vv := m[key] if vv == nil { lim -= int64(len(key)) - lim -= 100 // map entry overhead + lim -= mapEntryOverhead } lim -= int64(len(value)) if lim < 0 { |