diff --git a/file.go b/file.go index 2a85379a..0420db94 100644 --- a/file.go +++ b/file.go @@ -5,4 +5,10 @@ package runtime import "github.com/go-openapi/swag/fileutils" +// File represents an uploaded file. Re-exported from +// [fileutils.File] for backwards compatibility. +// +// See [BindForm] (in form.go) for the orchestrator that parses +// multipart / urlencoded request bodies and binds declared file +// fields onto handler-side targets. type File = fileutils.File diff --git a/form.go b/form.go new file mode 100644 index 00000000..fe5333b3 --- /dev/null +++ b/form.go @@ -0,0 +1,291 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package runtime + +import ( + stderrors "errors" + "fmt" + "mime/multipart" + "net/http" + + "github.com/go-openapi/errors" +) + +// DefaultMaxUploadFilenameLength is the default cap applied to +// FileHeader.Filename for each declared file when BindForm is invoked +// without an explicit [BindFormMaxFilenameLen] option. +// +// Multipart headers are allocated per part; an attacker submitting +// multi-MB filenames inflates the parser's memory footprint. 1 KiB +// matches the IETF guidance for sane filename length and is enough +// for realistic uploads. +const DefaultMaxUploadFilenameLength = 1024 + +// DefaultMaxUploadBodySize limits the size of the body to upload forms to 32MB. +// +// Use an explicit [BindFormMaxBody] option to change this limit. +const DefaultMaxUploadBodySize = int64(32) << 20 + +// filenamePreviewLen caps the byte length of the FileHeader.Filename +// preview embedded as the ParseError.Value field when the helper +// rejects a too-long filename. +const filenamePreviewLen = 32 + +// FileBinder is the per-file callback invoked by [BindForm] when a +// declared file field is present. +// +// The callback is responsible for BOTH validating the file (size, MIME, etc.) AND assigning the bound +// file to its destination — typically using: +// +// o.FieldName = &runtime.File{Data: file, Header: header} +// +// Returning a non-nil error surfaces the error in BindForm's per-field +// accumulator. Errors from the binder flow through verbatim — the +// binder is expected to produce HTTP-aware errors (e.g. +// errors.ExceedsMaximum from go-openapi/validate). +type FileBinder func(file multipart.File, header *multipart.FileHeader) error + +// BindOption configures [BindForm]. The variadic style keeps simple +// call sites simple and lets new knobs (security caps, additional +// behaviour) be added without breaking the signature. +type BindOption func(*bindConfig) + +type bindConfig struct { + maxParseMemory int64 + maxBody int64 + maxFiles int + maxFilenameLen int + files []formFileSpec +} + +type formFileSpec struct { + name string + required bool + bind FileBinder +} + +// BindFormMaxParseMemory caps the in-memory portion of a multipart +// body. Bytes beyond this are spilled to temporary files on disk by +// the stdlib parser. 0 (the default) defers to the stdlib's 32 MB. +// +// This option does NOT cap total body bytes — see [BindFormMaxBody] +// for that. The default body cap ([DefaultMaxUploadBodySize] = 32 MB) +// is applied even when this option is not supplied, so out of the box +// BindForm is bounded; callers with stricter or looser requirements +// adjust via [BindFormMaxBody]. +func BindFormMaxParseMemory(n int64) BindOption { + return func(c *bindConfig) { c.maxParseMemory = n } +} + +// BindFormMaxBody caps the size of the body read from a http form before parsing. +// +// The limit is set to 32MB by default. This default limit is applied for any n=0. +// +// The limit is disabled for n<0, assuming the caller has already capped the body size upstream. +func BindFormMaxBody(n int64) BindOption { + return func(c *bindConfig) { c.maxBody = n } +} + +// BindFormMaxFiles rejects parses where the total number of file +// parts across all field names exceeds n. 0 (the default) means no +// cap. Exceeding the cap is a fatal error — [BindForm] returns +// fatal=true and no per-file binders run. +func BindFormMaxFiles(n int) BindOption { + return func(c *bindConfig) { c.maxFiles = n } +} + +// BindFormMaxFilenameLen rejects per-file headers whose Filename +// length exceeds n. 0 means no cap; the default applied when this +// option is not supplied is [DefaultMaxUploadFilenameLength]. The +// cap is a per-field bind error (non-fatal); other declared files +// still run. +func BindFormMaxFilenameLen(n int) BindOption { + return func(c *bindConfig) { c.maxFilenameLen = n } +} + +// BindFormFile declares a file field to bind under the given form +// name. If required is true and the field is absent, [BindForm] +// produces the per-field error +// +// errors.NewParseError(name, "formData", "", http.ErrMissingFile) +// +// If required is false, absence is silent (no error, no bind). +// +// The bind callback runs only when the field is present. It is the +// site where both validation and assignment happen — see [FileBinder]. +// +// FileHeader.Filename is attacker-controlled text; the binder MUST +// NOT use it directly as a filesystem path. The helper does not +// touch the filesystem. +func BindFormFile(name string, required bool, bind FileBinder) BindOption { + return func(c *bindConfig) { + c.files = append(c.files, formFileSpec{ + name: name, + required: required, + bind: bind, + }) + } +} + +// BindForm parses r as multipart/form-data, falling back to +// application/x-www-form-urlencoded when the request is not +// multipart. On success, r.MultipartForm and r.PostForm are populated; +// the caller can read non-file form values via [Values](r.Form) after +// the call returns. +// +// All errors produced by BindForm itself (parse failure, missing +// required field, cap exceeded) are *errors.ParseError values built +// via [errors.NewParseError], matching the untyped +// middleware/parameter.go path. Errors returned by per-file binders +// flow through verbatim — binders own their HTTP-aware error shape. +// +// Per-file binders declared via [BindFormFile] run in declaration +// order after a successful parse. Their errors are accumulated and +// returned wrapped in [errors.CompositeValidationError]; the caller +// typically appends the returned err to its own []error and continues +// with non-file parameter binding. +// +// Return semantics: +// +// - fatal=true, err!=nil: parse failure or a hard cap (e.g. +// [BindFormMaxFiles]) was exceeded. No per-file binders ran; the +// caller MUST return err immediately. +// - fatal=false, err!=nil: one or more per-file binders produced +// errors. The form parsed successfully; r.Form is populated. The +// caller appends err to its accumulator and continues. +// - fatal=false, err==nil: full success. +// +// fatal==true implies err!=nil. +// +// Defaults applied out of the box: +// +// - Total body bytes capped at [DefaultMaxUploadBodySize] (32 MB) +// via [http.MaxBytesReader]. Adjust with [BindFormMaxBody] +// (negative n disables, when the caller has already capped the +// body upstream). +// - FileHeader.Filename length capped at +// [DefaultMaxUploadFilenameLength]. Adjust with +// [BindFormMaxFilenameLen]. +// +// Caller responsibilities the helper does NOT cover: +// +// - Set http.Server.ReadTimeout / IdleTimeout to defend against +// slow-read attacks. +// - Decompress Content-Encoding: gzip request bodies upstream if +// the API accepts them, using a size-limited reader. +// - Treat FileHeader.Filename as untrusted user input; never use +// it directly as a filesystem path. +func BindForm(r *http.Request, opts ...BindOption) (fatal bool, err error) { + cfg := bindConfig{ + maxFilenameLen: DefaultMaxUploadFilenameLength, + } + for _, opt := range opts { + opt(&cfg) + } + + if perr := parseFormBody(r, cfg.maxParseMemory, cfg.maxBody); perr != nil { + // Body-cap hit gets the 413 status; everything else maps to a + // 400 ParseError. parseFormBody returns the raw stdlib error + // in both cases — the HTTP-aware wrapping happens here. + var maxBytesErr *http.MaxBytesError + if stderrors.As(perr, &maxBytesErr) { + return true, errors.New(http.StatusRequestEntityTooLarge, "formData: %v", perr) + } + return true, errors.NewParseError("body", "formData", "", perr) + } + + if cfg.maxFiles > 0 { + if got := countFileParts(r); got > cfg.maxFiles { + return true, errors.NewParseError("body", "formData", "", + fmt.Errorf("multipart form contains %d file parts, exceeds limit %d", got, cfg.maxFiles)) + } + } + + var bindErrs []error + for _, spec := range cfg.files { + if e := bindFormFile(r, spec, cfg.maxFilenameLen); e != nil { + bindErrs = append(bindErrs, e) + } + } + if len(bindErrs) > 0 { + return false, errors.CompositeValidationError(bindErrs...) + } + return false, nil +} + +// parseFormBody parses the request body. Content-Type drives the +// parser: multipart/form-data → r.ParseMultipartForm, everything else +// → r.ParseForm (stdlib's parsePostForm only actually reads the body +// when Content-Type is application/x-www-form-urlencoded, so calling +// ParseForm is safe for unrecognised types). +// +// Caveat: ParseMultipartForm calls ParseForm internally and discards its error +// when the body turns out not to be multipart, returning ErrNotMultipart instead +// — the subsequent retry then short-circuits because r.PostForm is already +// set. Content-type-based routing avoids the lossy detour. +// +// Returns the raw stdlib error on failure; the caller (BindForm) +// handles HTTP-aware wrapping (413 for MaxBytesError, 400 ParseError +// otherwise). +// +// maxMemory == 0 falls through to the stdlib default (32 MB). +// maxBody == 0 defaults to DefaultMaxUploadBodySize; maxBody < 0 +// disables the body cap (caller has capped upstream). +func parseFormBody(r *http.Request, maxMemory, maxBody int64) error { + if r.Body != nil && maxBody >= 0 { + if maxBody == 0 { + maxBody = DefaultMaxUploadBodySize + } + r.Body = http.MaxBytesReader(nil, r.Body, maxBody) + } + + mt, _, _ := ContentType(r.Header) + if mt == MultipartFormMime { + //nolint:gosec // G120: false positive (gosec doesn't track the Body). See https://github.com/securego/gosec/blob/de65614d10a6b84029e3e1215567b8ce7e490f23/testutils/g120_samples.go#L57 + return r.ParseMultipartForm(maxMemory) + } + return r.ParseForm() +} + +func countFileParts(r *http.Request) int { + if r.MultipartForm == nil { + return 0 + } + var n int + for _, fhs := range r.MultipartForm.File { + n += len(fhs) + } + + return n +} + +func bindFormFile(r *http.Request, spec formFileSpec, maxFilenameLen int) error { + file, header, err := r.FormFile(spec.name) + if err != nil { + if stderrors.Is(err, http.ErrMissingFile) { + if spec.required { + return errors.New(http.StatusBadRequest, "formData: %v", http.ErrMissingFile) + } + + return nil + } + + return errors.NewParseError(spec.name, "formData", "", err) + } + + if maxFilenameLen > 0 && len(header.Filename) > maxFilenameLen { + preview := header.Filename + if len(preview) > filenamePreviewLen { + preview = preview[:filenamePreviewLen] + } + return errors.NewParseError(spec.name, "formData", preview, + fmt.Errorf("filename length %d exceeds limit %d", len(header.Filename), maxFilenameLen)) + } + + if spec.bind == nil { + return nil + } + + return spec.bind(file, header) +} diff --git a/form_test.go b/form_test.go new file mode 100644 index 00000000..dab33943 --- /dev/null +++ b/form_test.go @@ -0,0 +1,412 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package runtime + +import ( + "bytes" + stderrors "errors" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-openapi/errors" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +const ( + testUploadPath = "/upload" + testFileFieldB = "b.txt" + testFileFieldOK = "ok.txt" + testFileFieldA = "a.txt" + testFileData = "data" + testFieldDesc = "desc" + testFieldFile = "file" + testFieldFile1 = "file1" + testFieldFile2 = "file2" + testValueHello = "hello" +) + +// multipartBody builds a multipart/form-data body with the given file +// parts ({name, filename, content}) and form values. Returns the body +// bytes and the Content-Type header to set on the request. +type multipartFile struct { + field string + filename string + content string +} + +func multipartBody(t *testing.T, files []multipartFile, values map[string]string) (*bytes.Buffer, string) { + t.Helper() + buf := &bytes.Buffer{} + w := multipart.NewWriter(buf) + for _, f := range files { + fw, err := w.CreateFormFile(f.field, f.filename) + require.NoError(t, err) + _, err = io.WriteString(fw, f.content) + require.NoError(t, err) + } + for k, v := range values { + require.NoError(t, w.WriteField(k, v)) + } + require.NoError(t, w.Close()) + return buf, w.FormDataContentType() +} + +func newMultipartRequest(t *testing.T, files []multipartFile, values map[string]string) *http.Request { + t.Helper() + body, ct := multipartBody(t, files, values) + r := httptest.NewRequestWithContext(t.Context(), http.MethodPost, testUploadPath, body) + r.Header.Set("Content-Type", ct) + return r +} + +func newURLEncodedRequest(t *testing.T, values map[string]string) *http.Request { + t.Helper() + form := make([]string, 0, len(values)) + for k, v := range values { + form = append(form, k+"="+v) + } + r := httptest.NewRequestWithContext(t.Context(), http.MethodPost, testUploadPath, strings.NewReader(strings.Join(form, "&"))) + r.Header.Set("Content-Type", URLencodedFormMime) + return r +} + +// assertParseError extracts a *errors.ParseError from err and checks +// its name/in fields plus an optional reason predicate. +func assertParseError(t *testing.T, err error, wantName string, reasonCheck func(error) bool) { + t.Helper() + require.Error(t, err) + var pe *errors.ParseError + require.True(t, stderrors.As(err, &pe), "expected *errors.ParseError, got %T", err) + assert.EqualT(t, wantName, pe.Name) + assert.EqualT(t, "formData", pe.In) + if reasonCheck != nil { + require.True(t, reasonCheck(pe.Reason), "reason check failed for %v", pe.Reason) + } +} + +// assertCompositeContains extracts a *errors.CompositeError from err +// and asserts that at least n inner errors satisfy match. +func assertCompositeContains(t *testing.T, err error, n int, match func(error) bool) { + t.Helper() + require.Error(t, err) + var ce *errors.CompositeError + require.True(t, stderrors.As(err, &ce), "expected *errors.CompositeError, got %T", err) + var got int + for _, e := range ce.Errors { + if match(e) { + got++ + } + } + assert.EqualT(t, n, got, "matched %d inner errors, want %d", got, n) +} + +func TestBindForm_parseOnly_multipart(t *testing.T) { + r := newMultipartRequest(t, nil, map[string]string{testFieldDesc: testValueHello}) + + fatal, err := BindForm(r) + + assert.FalseT(t, fatal) + require.NoError(t, err) + require.NotNil(t, r.MultipartForm) + assert.EqualT(t, testValueHello, r.Form.Get(testFieldDesc)) +} + +func TestBindForm_parseOnly_urlencoded(t *testing.T) { + r := newURLEncodedRequest(t, map[string]string{testFieldDesc: testValueHello, "count": "42"}) + + fatal, err := BindForm(r) + + assert.FalseT(t, fatal) + require.NoError(t, err) + assert.EqualT(t, testValueHello, r.PostForm.Get(testFieldDesc)) + assert.EqualT(t, "42", r.PostForm.Get("count")) +} + +func TestBindForm_parseFailure_urlencoded(t *testing.T) { + // Malformed URL escape in an urlencoded body. An earlier draft routed + // through ParseMultipartForm which silently swallowed this class of + // errors; this test guards against the regression. + r := httptest.NewRequestWithContext(t.Context(), http.MethodPost, testUploadPath, strings.NewReader("name=%3&age=32")) + r.Header.Set("Content-Type", URLencodedFormMime) + + fatal, err := BindForm(r) + + assert.TrueT(t, fatal) + assertParseError(t, err, "body", nil) +} + +func TestBindForm_parseFailure(t *testing.T) { + // Multipart Content-Type but truncated body — ParseMultipartForm fails. + body, ct := multipartBody(t, []multipartFile{{testFieldFile, "f.txt", testValueHello}}, nil) + truncated := body.Bytes()[:len(body.Bytes())-5] + r := httptest.NewRequestWithContext(t.Context(), http.MethodPost, testUploadPath, bytes.NewReader(truncated)) + r.Header.Set("Content-Type", ct) + + fatal, err := BindForm(r) + + assert.TrueT(t, fatal) + assertParseError(t, err, "body", nil) +} + +func TestBindForm_singleRequired_present(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, "hello.txt", testValueHello}}, nil) + + var bound *File + fatal, err := BindForm(r, BindFormFile(testFieldFile, true, func(f multipart.File, h *multipart.FileHeader) error { + bound = &File{Data: f, Header: h} + return nil + })) + + assert.FalseT(t, fatal) + require.NoError(t, err) + require.NotNil(t, bound) + assert.EqualT(t, "hello.txt", bound.Header.Filename) +} + +func TestBindForm_singleRequired_missing(t *testing.T) { + r := newMultipartRequest(t, nil, map[string]string{testFieldDesc: "x"}) + + called := false + fatal, err := BindForm(r, BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { + called = true + return nil + })) + + assert.FalseT(t, fatal) + assert.FalseT(t, called) + assertCompositeContains(t, err, 1, func(e error) bool { + var apiErr errors.Error + if !stderrors.As(e, &apiErr) { + return false + } + return apiErr.Code() == http.StatusBadRequest && + strings.Contains(apiErr.Error(), http.ErrMissingFile.Error()) + }) +} + +func TestBindForm_optional_missing(t *testing.T) { + r := newMultipartRequest(t, nil, map[string]string{testFieldDesc: "x"}) + + called := false + fatal, err := BindForm(r, BindFormFile(testFieldFile, false, func(_ multipart.File, _ *multipart.FileHeader) error { + called = true + return nil + })) + + assert.FalseT(t, fatal) + require.NoError(t, err) + assert.FalseT(t, called, "optional missing file should not invoke binder") +} + +func TestBindForm_optional_present(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, "hi.txt", "hi"}}, nil) + + called := false + fatal, err := BindForm(r, BindFormFile(testFieldFile, false, func(f multipart.File, h *multipart.FileHeader) error { + called = true + assert.EqualT(t, "hi.txt", h.Filename) + got, _ := io.ReadAll(f) + assert.EqualT(t, "hi", string(got)) + return nil + })) + + assert.FalseT(t, fatal) + require.NoError(t, err) + assert.TrueT(t, called) +} + +func TestBindForm_mixed_files_and_values(t *testing.T) { + r := newMultipartRequest(t, + []multipartFile{ + {testFieldFile1, testFileFieldA, "AAA"}, + {testFieldFile2, testFileFieldB, "BBBB"}, + }, + map[string]string{testFieldDesc: "two files", "count": "2"}, + ) + + var f1, f2 *File + fatal, err := BindForm(r, + BindFormFile(testFieldFile1, true, func(f multipart.File, h *multipart.FileHeader) error { + f1 = &File{Data: f, Header: h} + return nil + }), + BindFormFile(testFieldFile2, false, func(f multipart.File, h *multipart.FileHeader) error { + f2 = &File{Data: f, Header: h} + return nil + }), + ) + + assert.FalseT(t, fatal) + require.NoError(t, err) + require.NotNil(t, f1) + require.NotNil(t, f2) + assert.EqualT(t, testFileFieldA, f1.Header.Filename) + assert.EqualT(t, testFileFieldB, f2.Header.Filename) + assert.EqualT(t, "two files", r.Form.Get(testFieldDesc)) + assert.EqualT(t, "2", r.Form.Get("count")) +} + +func TestBindForm_binderError(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, "f.txt", testFileData}}, nil) + sentinel := stderrors.New("binder rejected") + + fatal, err := BindForm(r, BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { + return sentinel + })) + + assert.FalseT(t, fatal) + assertCompositeContains(t, err, 1, func(e error) bool { return stderrors.Is(e, sentinel) }) +} + +func TestBindForm_multipleBinderErrors(t *testing.T) { + r := newMultipartRequest(t, + []multipartFile{ + {testFieldFile1, testFileFieldA, "A"}, + {testFieldFile2, testFileFieldB, "B"}, + }, + nil, + ) + errA := stderrors.New("A failed") + errB := stderrors.New("B failed") + + fatal, err := BindForm(r, + BindFormFile(testFieldFile1, true, func(_ multipart.File, _ *multipart.FileHeader) error { return errA }), + BindFormFile(testFieldFile2, true, func(_ multipart.File, _ *multipart.FileHeader) error { return errB }), + ) + + assert.FalseT(t, fatal) + assertCompositeContains(t, err, 1, func(e error) bool { return stderrors.Is(e, errA) }) + assertCompositeContains(t, err, 1, func(e error) bool { return stderrors.Is(e, errB) }) +} + +func TestBindForm_maxFiles_exceeded(t *testing.T) { + r := newMultipartRequest(t, + []multipartFile{ + {testFieldFile1, testFileFieldA, "A"}, + {testFieldFile2, testFileFieldB, "B"}, + {"file3", "c.txt", "C"}, + }, + nil, + ) + + bound := 0 + fatal, err := BindForm(r, + BindFormMaxFiles(2), + BindFormFile(testFieldFile1, false, func(_ multipart.File, _ *multipart.FileHeader) error { + bound++ + return nil + }), + ) + + assert.TrueT(t, fatal) + assertParseError(t, err, "body", nil) + assert.EqualT(t, 0, bound, "no binders should run after maxFiles exceeded") +} + +func TestBindForm_maxFilenameLen_exceeded(t *testing.T) { + longName := strings.Repeat("x", 50) + ".txt" + r := newMultipartRequest(t, + []multipartFile{ + {"big", longName, testFileData}, + {"small", testFileFieldOK, testFileData}, + }, + nil, + ) + + smallBound := false + fatal, err := BindForm(r, + BindFormMaxFilenameLen(10), + BindFormFile("big", true, func(_ multipart.File, _ *multipart.FileHeader) error { + return nil + }), + BindFormFile("small", false, func(_ multipart.File, _ *multipart.FileHeader) error { + smallBound = true + return nil + }), + ) + + assert.FalseT(t, fatal) + assertCompositeContains(t, err, 1, func(e error) bool { + var pe *errors.ParseError + if !stderrors.As(e, &pe) { + return false + } + return pe.Name == "big" + }) + assert.TrueT(t, smallBound, "small file should still bind after big rejected") +} + +func TestBindForm_maxMemory_zero(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, testFileFieldOK, testValueHello}}, nil) + + fatal, err := BindForm(r, + BindFormMaxParseMemory(0), + BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { return nil }), + ) + + assert.FalseT(t, fatal) + require.NoError(t, err) +} + +func TestBindForm_maxBody_underCap(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, testFileFieldOK, testFileData}}, nil) + + fatal, err := BindForm(r, + BindFormMaxBody(1<<20), + BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { return nil }), + ) + + assert.FalseT(t, fatal) + require.NoError(t, err) +} + +func TestBindForm_maxBody_overCap(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, testFileFieldOK, strings.Repeat("x", 2048)}}, nil) + + fatal, err := BindForm(r, + BindFormMaxBody(256), + BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { return nil }), + ) + + assert.TrueT(t, fatal) + require.Error(t, err) + var apiErr errors.Error + require.True(t, stderrors.As(err, &apiErr), "expected errors.Error, got %T", err) + assert.EqualT(t, int32(http.StatusRequestEntityTooLarge), apiErr.Code()) +} + +func TestBindForm_maxBody_disabled(t *testing.T) { + // Body well above DefaultMaxUploadBodySize would be expensive; just + // confirm a 2 MiB body parses with n=-1 (disabled) when the implicit + // default would otherwise stay at 32 MiB anyway. The point is that + // passing -1 doesn't itself break parsing. + r := newMultipartRequest(t, []multipartFile{{testFieldFile, testFileFieldOK, strings.Repeat("x", 2<<20)}}, nil) + + fatal, err := BindForm(r, + BindFormMaxBody(-1), + BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { return nil }), + ) + + assert.FalseT(t, fatal) + require.NoError(t, err) +} + +func TestBindForm_idempotent(t *testing.T) { + r := newMultipartRequest(t, []multipartFile{{testFieldFile, testFileFieldOK, testValueHello}}, map[string]string{testFieldDesc: "x"}) + + fatal1, err1 := BindForm(r, BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { return nil })) + require.NoError(t, err1) + assert.FalseT(t, fatal1) + + // Re-call: stdlib short-circuits because r.MultipartForm != nil. + fatal2, err2 := BindForm(r, BindFormFile(testFieldFile, true, func(_ multipart.File, _ *multipart.FileHeader) error { return nil })) + require.NoError(t, err2) + assert.FalseT(t, fatal2) + assert.EqualT(t, "x", r.Form.Get(testFieldDesc)) +} diff --git a/middleware/parameter.go b/middleware/parameter.go index 224fdd73..d0108424 100644 --- a/middleware/parameter.go +++ b/middleware/parameter.go @@ -90,17 +90,8 @@ func (p *untypedParamBinder) Bind(request *http.Request, routeParams RouteParams return p.bindValue(data, hasKey, target) case "formData": - var err error - var mt string - - mt, _, e := runtime.ContentType(request.Header) - if e != nil { - // because of the interface conversion go thinks the error is not nil - // so we first check for nil and then set the err var if it's not nil - err = e - } - - if err != nil { + mt, _, ctErr := runtime.ContentType(request.Header) + if ctErr != nil { return errors.InvalidContentType("", []string{runtime.MultipartFormMime, runtime.URLencodedFormMime}) } @@ -108,14 +99,13 @@ func (p *untypedParamBinder) Bind(request *http.Request, routeParams RouteParams return errors.InvalidContentType(mt, []string{runtime.MultipartFormMime, runtime.URLencodedFormMime}) } - if mt == runtime.MultipartFormMime { - if err = request.ParseMultipartForm(defaultMaxMemory); err != nil { - return errors.NewParseError(p.Name, p.parameter.In, "", err) - } - } - - if err = request.ParseForm(); err != nil { - return errors.NewParseError(p.Name, p.parameter.In, "", err) + // Parse via the shared helper. The helper routes on Content-Type + // (multipart/form-data → ParseMultipartForm, otherwise → ParseForm) + // and applies the default 32 MiB body cap via http.MaxBytesReader. + // Idempotent across the per-parameter loop: stdlib short-circuits + // when r.MultipartForm / r.PostForm are already populated. + if _, perr := runtime.BindForm(request, runtime.BindFormMaxParseMemory(defaultMaxMemory)); perr != nil { + return perr } if p.parameter.Type == "file" {