From a23c2e510e134a8cddb2a156fd8733fbdc12a042 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Thu, 14 May 2026 16:20:24 +0200 Subject: [PATCH] feat(runtime): BindForm helper for multipart/urlencoded body binding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a single orchestrator helper to the root runtime package that dedupes the parse-and-bind dance go-swagger codegen emits for every operation with form-data parameters: - BindForm(r, opts...) (fatal bool, err error) — parses the body (multipart/form-data, fallback to application/x-www-form-urlencoded) and runs per-file binders declared via BindFormFile options. - BindFormFile(name, required, FileBinder) — declares a file field. - BindFormMaxParseMemory, BindFormMaxBody, BindFormMaxFiles, BindFormMaxFilenameLen — security caps injectable via options. The (fatal, err) return preserves the codegen pattern of bailing on parse failure but accumulating per-file validation errors into a composite. Body cap defaults to 32 MiB via http.MaxBytesReader so the helper is bounded out of the box (413 on overflow); filename cap defaults to 1 KiB. All helper-produced errors are *errors.ParseError values built via errors.NewParseError, matching the untyped middleware/parameter.go path. Already-structured errors.Error values (e.g. 413 from a MaxBytesReader hit) pass through with their original code. Binder errors flow through verbatim — binders own their HTTP-aware shape. Pure addition to this module. The go-swagger codegen change consuming the helper is a separate PR in that repo. refactor(middleware): untyped formData binder uses runtime.BindForm Consolidate the multipart/urlencoded parse dance in middleware/parameter.go's "formData" case onto runtime.BindForm. The pre-flight content-type check (errors.InvalidContentType) and the per-file FormFile block stay open-coded; the manual ParseMultipartForm/ParseForm dance plus the err-wrap dance collapse into one helper call. Untyped error shapes are byte-identical to before; body is now capped at 32 MiB by default via http.MaxBytesReader (helper default). The helper also unifies errors reported by both the codegen use case and the untyped use case. Co-Authored-By: Claude Opus 4.7 Signed-off-by: Frederic BIDON --- file.go | 6 + form.go | 291 ++++++++++++++++++++++++++++ form_test.go | 412 ++++++++++++++++++++++++++++++++++++++++ middleware/parameter.go | 28 +-- 4 files changed, 718 insertions(+), 19 deletions(-) create mode 100644 form.go create mode 100644 form_test.go 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" {