diff --git a/client/request.go b/client/request.go index f16ee487..0ad2da44 100644 --- a/client/request.go +++ b/client/request.go @@ -189,6 +189,14 @@ func (r *request) SetTimeout(timeout time.Duration) error { } func (r *request) isMultipart(mediaType string) bool { + // An explicit application/x-www-form-urlencoded choice is honored even when + // file fields are present: the spec allows files to travel as URL-encoded + // form values, although it does not stream and is discouraged. Without this + // short-circuit, picking urlencoded with files would silently fall back to + // multipart and emit an inconsistent Content-Type. + if strings.EqualFold(mediaType, runtime.URLencodedFormMime) { + return false + } if len(r.fileFields) > 0 { return true } @@ -223,8 +231,29 @@ func (r *request) buildHTTP(mediaType, basePath string, producers map[string]run if len(r.formFields) > 0 || len(r.fileFields) > 0 { if !r.isMultipart(mediaType) { r.header.Set(runtime.HeaderContentType, mediaType) - formString := r.formFields.Encode() - r.buf.WriteString(formString) + values := url.Values{} + for k, vs := range r.formFields { + values[k] = append(values[k], vs...) + } + // Per Swagger 2.0, file form parameters can be sent under + // application/x-www-form-urlencoded by including the file content as a + // regular form-field value. The whole form is then percent-encoded as + // usual. This buffers the entire payload and does not preserve a + // per-file Content-Type — multipart/form-data is preferred when both + // are advertised by the operation. + for fn, ff := range r.fileFields { + for _, fi := range ff { + data, ferr := io.ReadAll(fi) + if cerr := fi.Close(); cerr != nil && ferr == nil { + ferr = cerr + } + if ferr != nil { + return nil, ferr + } + values.Add(fn, string(data)) + } + } + r.buf.WriteString(values.Encode()) goto DoneChoosingBodySource } @@ -460,9 +489,6 @@ func logClose(err error, pw *io.PipeWriter) { } } -func mangleContentType(mediaType, boundary string) string { - if strings.ToLower(mediaType) == runtime.URLencodedFormMime { - return fmt.Sprintf("%s; boundary=%s", mediaType, boundary) - } +func mangleContentType(_, boundary string) string { return "multipart/form-data; boundary=" + boundary } diff --git a/client/request_test.go b/client/request_test.go index 6376e88f..4c5ea20d 100644 --- a/client/request_test.go +++ b/client/request_test.go @@ -450,6 +450,10 @@ func TestBuildRequest_BuildHTTP_Files(t *testing.T) { fileverifier("empty", 0, filepath.Base(emptyFile.Name()), []byte{}) } +// TestBuildRequest_BuildHTTP_Files_URLEncoded covers issue #286: when the +// caller explicitly picks application/x-www-form-urlencoded, file fields must +// be encoded as regular URL-encoded form values rather than producing a +// multipart body advertised under a urlencoded Content-Type. func TestBuildRequest_BuildHTTP_Files_URLEncoded(t *testing.T) { cont, err := os.ReadFile("./runtime.go") require.NoError(t, err) @@ -474,31 +478,23 @@ func TestBuildRequest_BuildHTTP_Files_URLEncoded(t *testing.T) { assert.EqualT(t, "200", req.Header.Get("X-Rate-Limit")) assert.EqualT(t, "world", req.URL.Query().Get("hello")) assert.EqualT(t, "/flats/1234/", req.URL.Path) - mediaType, params, err := mime.ParseMediaType(req.Header.Get(runtime.HeaderContentType)) - require.NoError(t, err) - assert.EqualT(t, runtime.URLencodedFormMime, mediaType) - boundary := params["boundary"] - mr := multipart.NewReader(req.Body, boundary) - defer req.Body.Close() - frm, err := mr.ReadForm(1 << 20) + // Content-Type must be the bare urlencoded type — no boundary parameter. + assert.EqualT(t, runtime.URLencodedFormMime, req.Header.Get(runtime.HeaderContentType)) + + body, err := io.ReadAll(req.Body) require.NoError(t, err) + defer req.Body.Close() - assert.EqualT(t, "some value", frm.Value["something"][0]) - fileverifier := func(name string, index int, filename string, content []byte) { - mpff := frm.File[name][index] - mpf, e := mpff.Open() - require.NoError(t, e) - defer mpf.Close() - assert.EqualT(t, filename, mpff.Filename) - actual, e := io.ReadAll(mpf) - require.NoError(t, e) - assert.Equal(t, content, actual) - } - fileverifier("file", 0, "runtime.go", cont) + values, err := url.ParseQuery(string(body)) + require.NoError(t, err) - fileverifier("otherfiles", 0, "runtime.go", cont) - fileverifier("otherfiles", 1, "request.go", cont2) + assert.EqualT(t, "some value", values.Get("something")) + require.Len(t, values["file"], 1) + assert.Equal(t, string(cont), values["file"][0]) + require.Len(t, values["otherfiles"], 2) + assert.Equal(t, string(cont), values["otherfiles"][0]) + assert.Equal(t, string(cont2), values["otherfiles"][1]) } type contentTypeProvider struct { diff --git a/client/runtime.go b/client/runtime.go index eeb17dfb..f41818ea 100644 --- a/client/runtime.go +++ b/client/runtime.go @@ -545,14 +545,7 @@ func (r *Runtime) createHttpRequest(operation *runtime.ClientOperation) (*reques //} // Enhancement proposal: https://github.com/go-openapi/runtime/issues/386 - cmt := r.DefaultMediaType - for _, mediaType := range operation.ConsumesMediaTypes { - // Pick first non-empty media type - if mediaType != "" { - cmt = mediaType - break - } - } + cmt := pickConsumesMediaType(operation.ConsumesMediaTypes, r.DefaultMediaType) if _, ok := r.Producers[cmt]; !ok && cmt != runtime.MultipartFormMime && cmt != runtime.URLencodedFormMime { return nil, nil, fmt.Errorf("none of producers: %v registered. try %s", r.Producers, cmt) @@ -568,6 +561,27 @@ func (r *Runtime) createHttpRequest(operation *runtime.ClientOperation) (*reques return request, req, nil } +// pickConsumesMediaType selects which Content-Type the client will send. +// +// When the operation advertises multipart/form-data alongside other types +// (typically application/x-www-form-urlencoded), multipart is preferred because +// it streams and preserves per-file Content-Type. Otherwise the first non-empty +// entry wins, falling back to def. This makes the choice independent of the +// order produced by codegen. +func pickConsumesMediaType(consumes []string, def string) string { + for _, mt := range consumes { + if strings.EqualFold(mt, runtime.MultipartFormMime) { + return mt + } + } + for _, mt := range consumes { + if mt != "" { + return mt + } + } + return def +} + func basePool(pool *x509.CertPool) *x509.CertPool { if pool == nil { return x509.NewCertPool() diff --git a/client/runtime_test.go b/client/runtime_test.go index a188e007..e6ce5b40 100644 --- a/client/runtime_test.go +++ b/client/runtime_test.go @@ -476,6 +476,38 @@ func TestRuntime_AuthCanary(t *testing.T) { assert.Equal(t, result, actual) } +// TestPickConsumesMediaType covers the selection rule used by the client +// runtime: when an operation advertises both multipart/form-data and +// application/x-www-form-urlencoded, multipart wins regardless of the order +// codegen produced. See issue #286. +func TestPickConsumesMediaType(t *testing.T) { + const def = "application/json" + cases := []struct { + name string + consumes []string + want string + }{ + {"empty falls back to default", nil, def}, + {"only empties fall back to default", []string{"", ""}, def}, + {"single entry wins", []string{"text/plain"}, "text/plain"}, + {"first non-empty wins when no multipart", + []string{"", runtime.URLencodedFormMime}, runtime.URLencodedFormMime}, + {"multipart preferred when listed first", + []string{runtime.MultipartFormMime, runtime.URLencodedFormMime}, runtime.MultipartFormMime}, + {"multipart preferred when listed last", + []string{runtime.URLencodedFormMime, runtime.MultipartFormMime}, runtime.MultipartFormMime}, + {"caller's explicit single choice is honored", + []string{runtime.URLencodedFormMime}, runtime.URLencodedFormMime}, + {"multipart match is case-insensitive", + []string{runtime.URLencodedFormMime, "Multipart/Form-Data"}, "Multipart/Form-Data"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.EqualT(t, tc.want, pickConsumesMediaType(tc.consumes, def)) + }) + } +} + func TestRuntime_PickConsumer(t *testing.T) { result := []task{ {false, "task 1 content", 1},