Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions client/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
38 changes: 17 additions & 21 deletions client/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
30 changes: 22 additions & 8 deletions client/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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()
Expand Down
32 changes: 32 additions & 0 deletions client/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
Loading