400 on a multipart/form-data request with an empty body · Issue #1361 · userver-framework/userver · GitHub
Skip to content

400 on a multipart/form-data request with an empty body #1361

Open

Description

@SSE4

Add a description

A request with a multipart/form-data Content-Type and a zero-length body
(Content-Length: 0) is rejected with
400 invalid body of multipart/form-data request, before any handler runs.
The request is well-formed HTTP in both HTTP/1.1 and HTTP/2; nothing in the HTTP
specs requires a server to reject it, and 400 is the wrong code for the problem
it does have.

Version

Reproduced on develop @ ad3c7b1ff. The parser file has not changed since
7d7b2f400 (tagged v2.14).

Steps to reproduce

Unit level:

TEST(MultipartFormDataParser, ParseEmptyBody) {
    namespace sh = server::http;
    sh::FormDataArgs form_data_args;
    EXPECT_TRUE(ParseMultipartFormData("multipart/form-data; boundary=zzz", "", form_data_args));  // fails today
    EXPECT_TRUE(form_data_args.empty());
}

HTTP level:

POST /v1/multipart HTTP/1.1
Host: localhost
Content-Type: multipart/form-data; boundary=zzz
Content-Length: 0

Expected: the handler runs and decides for itself (the
samples/multipart_service handler would return its own 400 once
GetFormDataArg("profileImage") comes back empty — which is the correct place
for that decision).
Actual: 400 Bad Request from the framework, log line
Unexpected request body end. The handler never runs.

Note the contrast with an empty form, --zzz--\r\n, which userver already
accepts with 200. Only the zero-byte case fails.

Root cause

core/src/server/http/multipart_form_data_parser.cpp:338-419,
ParseMultipartFormDataBody. There is no zero-length case: an empty body fails
the leading-boundary check at :347, falls into the skip-preamble branch, and
FindBoundaryEnd returns npos → LOG_WARNING() << "Unexpected request body end"; return false; (:361-365).

Note that an empty form — a body of exactly --boundary-- — is already
accepted (ParseEmptyForm, and NoFinalCrLf2 without the trailing CRLF). So the
parser already has a notion of "valid request, no form arguments"; the zero-byte
case is simply not routed to it.

Spec references

HTTP says the request is valid. RFC 9112 §6: message-body = *OCTET, and
"The presence of a message body in a request is signaled by a Content-Length or
Transfer-Encoding header field. Request message framing is independent of method
semantics."
RFC 9113 §8.1.1 enumerates malformed HTTP/2 messages exhaustively
(extraneous frames, prohibited or absent pseudo-header fields, invalid field
names, content-length ≠ sum of DATA payload lengths) — a zero content-length
with no DATA frames matches none of them, and content-type does not appear in
the definition at all. No HTTP spec requires content to conform to the grammar
of its declared media type.

MIME says the content is malformed. RFC 2046 §5.1.1:

multipart-body := [preamble CRLF]
                  dash-boundary transport-padding CRLF
                  body-part *encapsulation
                  close-delimiter transport-padding
                  [CRLF epilogue]

Only *encapsulation is zero-or-more; dash-boundary, one body-part and
close-delimiter are mandatory, so a zero-byte body cannot match. That much is
real — but RFC 2046 defines what the content is, not what an HTTP server must
answer.

Rejecting is permitted, not required, and 400 is the wrong code. RFC 9110
§2.4: "Unless noted otherwise, a recipient MAY attempt to recover a usable
protocol element from an invalid construct. HTTP does not define specific error
handling mechanisms except when they have a direct impact on security, since
different applications of the protocol require different error handling
strategies."
And if a server does reject, §15.5.1 scopes 400 to "malformed
request syntax, invalid request message framing, or deceptive request routing"

— all HTTP-level concerns. Content-format problems have their own codes: 415
(§15.5.16, "the content is in a format not supported... might be due to the
request's indicated Content-Type... or as a result of inspecting the data
directly"
) and 422 (§15.5.21).

Proposed fix

Early-return an empty body as an empty form in ParseMultipartFormDataBody:

// A zero-length body is not a valid multipart entity (RFC 2046 5.1.1), but it
// is a valid HTTP request. Surface it to the handler as an empty form rather
// than failing the request -- whether a missing form is an error is the
// handler's decision.
if (body.empty()) return true;

Handlers that require form arguments are unaffected: GetFormDataArg returns
empty and they can answer 415/422/400 themselves with a message that actually
describes their contract.

If reviewers prefer to keep rejecting, note that 400 is the wrong code either
way — §15.5.1 scopes it to HTTP syntax, framing and routing, while content-format
problems have 415 (§15.5.16) and 422 (§15.5.21). But swapping 400 for 415 only
makes the framework wrong more precisely: the status still belongs to the
handler, not to the request constructor. See
[[userver-multipart-parse-failure-fatal-issue]], which removes the question by
making a parse failure non-fatal. The empty-form reading here remains the
recommendation regardless of whether that one lands.

Scope note

This ticket covers only the zero-length body. Other malformed bodies (garbage,
truncated, missing final boundary) keep failing, and the same 400-vs-415
argument applies to them — but changing those is a behaviour change for real
malformed input, which belongs in
[[userver-multipart-parse-failure-fatal-issue]] rather than here.

Tests

  • ParseEmptyBody (unit) — an empty body parses to an empty form, for all three
    strict_cr_lf settings (default, true, false).
  • ParseEmptyBodyLeniencyIsNarrow (unit) — a Content-Type with no boundary,
    a whitespace-only body ("\r\n") and a truncated one ("--zzz\r\n") all still
    fail, so the leniency cannot widen unnoticed.
  • test_empty_body_is_rejected_by_the_handler (functional,
    samples/multipart_service) — posting an empty body now gets the handler's own
    Expecting PNG image format rather than the framework's
    invalid body of multipart/form-data request.

Verified on the branch: ParseEmptyBody and the functional test fail without the
production change and pass with it; ParseEmptyBodyLeniencyIsNarrow passes
either way. Full core unit suite 2218/2219 (one pre-existing skip) and all 7
basic-chaos suites stay green.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions