From 725fd5fc33e6b7cbfc6afae191a3daa1bda3e290 Mon Sep 17 00:00:00 2001 From: Matt Moyles Date: Mon, 19 Oct 2026 09:25:40 -0400 Subject: [PATCH] Return a clear error when a multipart form has no file When a multi-part form ends without any part that has a filename, NextPart returns io.EOF and getFilePart wrapped it as FF00161 "Error reading multi-part form input: EOF". That reads like a broken upload, when the real problem is that no file was sent, for example a file part with an empty filename. Return a new 400 error, FF00262, for that case. Other read failures, including a body that ends before the closing boundary, still return FF00161. Refs hyperledger-firefly/firefly#220 Signed-off-by: Matt Moyles --- pkg/ffapi/handler.go | 5 ++ pkg/ffapi/handler_test.go | 108 +++++++++++++++++++++++++++++ pkg/i18n/en_base_error_messages.go | 1 + 3 files changed, 114 insertions(+) diff --git a/pkg/ffapi/handler.go b/pkg/ffapi/handler.go index e2bdc66..24bd87c 100644 --- a/pkg/ffapi/handler.go +++ b/pkg/ffapi/handler.go @@ -106,6 +106,11 @@ func (hs *HandlerFactory) getFilePart(req *http.Request) (*multipartState, error } for { part, err := mpr.NextPart() + // Not errors.Is: NextPart returns a bare io.EOF only at the final boundary, while a + // truncated body comes back wrapped and should keep the FF00161 read error + if err == io.EOF { + return nil, i18n.NewError(ctx, i18n.MsgMultiPartFormNoFile) + } if err != nil { return nil, i18n.WrapError(ctx, err, i18n.MsgMultiPartFormReadError) } diff --git a/pkg/ffapi/handler_test.go b/pkg/ffapi/handler_test.go index 4fced88..5330639 100644 --- a/pkg/ffapi/handler_test.go +++ b/pkg/ffapi/handler_test.go @@ -565,6 +565,114 @@ func TestMultipartBadData(t *testing.T) { assert.Regexp(t, "FF00161", resJSON["error"]) } +func TestMultipartNoFilename(t *testing.T) { + s, _, done := newTestServer(t, []*Route{{ + Name: "testRoute", + Path: "/test", + Method: "POST", + FormParams: []*FormParam{ + {Name: "param1", Description: ExampleDesc}, + }, + JSONInputValue: nil, + JSONOutputValue: func() interface{} { return make(map[string]interface{}) }, + JSONOutputCodes: []int{http.StatusCreated}, + FormUploadHandler: func(r *APIRequest) (output interface{}, err error) { + return nil, nil + }, + }}, "", nil) + defer done() + + b := new(bytes.Buffer) + w := multipart.NewWriter(b) + writer, err := w.CreateFormField("param1") + assert.NoError(t, err) + writer.Write([]byte(`testing`)) + writer, err = w.CreateFormFile("file", "") + assert.NoError(t, err) + writer.Write([]byte(`some data`)) + w.Close() + + req, err := http.NewRequest("POST", fmt.Sprintf("http://%s/test", s.Addr()), b) + assert.NoError(t, err) + req.Header.Set("Content-Type", w.FormDataContentType()) + res, err := http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, 400, res.StatusCode) + var resJSON map[string]interface{} + json.NewDecoder(res.Body).Decode(&resJSON) + assert.Regexp(t, "FF00262", resJSON["error"]) +} + +func TestMultipartNoFilePart(t *testing.T) { + s, _, done := newTestServer(t, []*Route{{ + Name: "testRoute", + Path: "/test", + Method: "POST", + FormParams: []*FormParam{ + {Name: "param1", Description: ExampleDesc}, + }, + JSONInputValue: nil, + JSONOutputValue: func() interface{} { return make(map[string]interface{}) }, + JSONOutputCodes: []int{http.StatusCreated}, + FormUploadHandler: func(r *APIRequest) (output interface{}, err error) { + return nil, nil + }, + }}, "", nil) + defer done() + + b := new(bytes.Buffer) + w := multipart.NewWriter(b) + writer, err := w.CreateFormField("param1") + assert.NoError(t, err) + writer.Write([]byte(`testing`)) + w.Close() + + req, err := http.NewRequest("POST", fmt.Sprintf("http://%s/test", s.Addr()), b) + assert.NoError(t, err) + req.Header.Set("Content-Type", w.FormDataContentType()) + res, err := http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, 400, res.StatusCode) + var resJSON map[string]interface{} + json.NewDecoder(res.Body).Decode(&resJSON) + assert.Regexp(t, "FF00262", resJSON["error"]) +} + +func TestMultipartTruncated(t *testing.T) { + s, _, done := newTestServer(t, []*Route{{ + Name: "testRoute", + Path: "/test", + Method: "POST", + FormParams: []*FormParam{ + {Name: "param1", Description: ExampleDesc}, + }, + JSONInputValue: nil, + JSONOutputValue: func() interface{} { return make(map[string]interface{}) }, + JSONOutputCodes: []int{http.StatusCreated}, + FormUploadHandler: func(r *APIRequest) (output interface{}, err error) { + return nil, nil + }, + }}, "", nil) + defer done() + + b := new(bytes.Buffer) + w := multipart.NewWriter(b) + writer, err := w.CreateFormField("param1") + assert.NoError(t, err) + writer.Write([]byte(`testing`)) + // No w.Close(), so the body ends without a closing boundary + + req, err := http.NewRequest("POST", fmt.Sprintf("http://%s/test", s.Addr()), b) + assert.NoError(t, err) + req.Header.Set("Content-Type", w.FormDataContentType()) + res, err := http.DefaultClient.Do(req) + assert.NoError(t, err) + assert.Equal(t, 400, res.StatusCode) + var resJSON map[string]interface{} + json.NewDecoder(res.Body).Decode(&resJSON) + assert.Regexp(t, "FF00161", resJSON["error"]) +} + func TestTextPlain201(t *testing.T) { s, _, done := newTestServer(t, []*Route{{ Name: "testRoute", diff --git a/pkg/i18n/en_base_error_messages.go b/pkg/i18n/en_base_error_messages.go index b251b30..bdeaccb 100644 --- a/pkg/i18n/en_base_error_messages.go +++ b/pkg/i18n/en_base_error_messages.go @@ -196,4 +196,5 @@ var ( MsgFFExtensionsInvalidEncoding = ffe("FF00259", "Invalid extension key '%s' - extension keys must follow the format 'x-'", http.StatusBadRequest) MsgInvalidCIDR = ffe("FF00260", "Invalid CIDR '%s' in denylist configuration", http.StatusBadRequest) MsgConnectionToCIDRBlocked = ffe("FF00261", "Connection to '%s' blocked by CIDR denylist (%s)", http.StatusForbidden) + MsgMultiPartFormNoFile = ffe("FF00262", "No file found in multi-part form input. The file part must have a filename", http.StatusBadRequest) )