Skip to content

feat: add multipart/form-data request body#146

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
feat/multipart-request-body
Open

feat: add multipart/form-data request body#146
OmarAlJarrah wants to merge 1 commit into
mainfrom
feat/multipart-request-body

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Adds MultipartBody, a multipart/form-data (RFC 7578) RequestBody for form and file uploads, to sdk-core.

What

  • MultipartBody extends RequestBody; MultipartBody.Part models a single field (Content-Disposition from name + optional filename, optional Content-Type, and a body). A MultipartBody.Builder (implementing Builder<MultipartBody>) assembles parts and generates a boundary.
  • mediaType() returns multipart/form-data; boundary=<token>. The boundary is a random token by default (dexpace- prefix + 32 token chars) and is overridable via builder().boundary(...).
  • File parts stream off disk through the existing FileRequestBody (so a transport that recognises FileRequestBody can still take its zero-copy path for the file region). Value parts are encoded through the Serde SPI at construction time, so no serializer is needed at send time and sdk-core keeps zero embedded-serialization deps (no Jackson in core).
  • The body is replayable when every part body is replayable, and otherwise falls back to the base toReplayable buffering. Part names and filenames are escaped (", CR, LF percent-encoded) so a value can never break the header framing.

Frame size cannot drift

writeTo and contentLength both go through a single shared frame function (frameOf for each part plus closingDelimiter). contentLength sums exactly the bytes those functions produce, so the declared length and the bytes actually written are computed from the same source and cannot diverge. A test asserts contentLength equals the byte count produced by writeTo for value-only and mixed file+value bodies.

Tests

MultipartBodyTest covers: boundary generation/uniqueness, media-type shape, exact contentLength == writeTo byte count (value and mixed file parts), unknown-length propagation, full multi-part wire-format round trip, file-part filename/Content-Type emission, single closing delimiter, name/filename escaping, replayability of all-replayable bodies, buffering fallback for a non-replayable part, and Serde-driven value parts.

Gated build

Run from the worktree with the module-scoped gates:

./gradlew :sdk-core:test :sdk-core:ktlintCheck :sdk-core:detekt :sdk-core:apiCheck --no-daemon

Result: BUILD SUCCESSFUL (all green). :sdk-core:apiDump was run and the regenerated sdk-core/api/sdk-core.api is committed.

Closes #61

Add MultipartBody, a multipart/form-data (RFC 7578) RequestBody for
form and file uploads, plus MultipartBody.Part and a builder.

- A single shared frame function (frameOf/closingDelimiter) backs both
  writeTo and contentLength, so the declared length cannot drift from
  the bytes actually written.
- File parts stream off disk via FileRequestBody; value parts are
  encoded through the Serde SPI at construction time, keeping sdk-core
  free of any embedded serializer.
- mediaType() reports multipart/form-data with the generated boundary
  parameter; the boundary is a random token, overridable on the builder.
- The body is replayable when every part body is replayable, and falls
  back to buffering otherwise. Part names and filenames are escaped so a
  value can never break the header framing.
@OmarAlJarrah

Copy link
Copy Markdown
Member Author

This adds a multipart/form-data (RFC 7578) RequestBody to sdk-core: a Part model with file/serialized/raw factories, a Builder, a SecureRandom-generated boundary, and a single frameOf function shared by writeTo and contentLength so the declared length can't drift from the bytes actually written. The framing, the content-length/writeTo equality, replayability, and the RFC 7578 quoting/escaping of part names and filenames all look solid and are well covered by tests. A couple of small things worth tightening before merge, neither blocking.

Issues

User-supplied boundary isn't validated for CR/LF or --MultipartBody.kt:230 (Builder.boundary). The builder only checks the boundary is non-empty, but it then flows unescaped into two places: the Content-Type header via MediaType.of(...) (whose formatParameterValue quotes but does not escape CR/LF), and directly into the body framing via frameOf/closingDelimiter (~:130-150) as --<boundary>. A boundary containing CRLF or -- can therefore split the framing and inject arbitrary part headers or body content. The generated boundary is always token-safe and this input is normally developer-controlled, so the practical risk is low — but the part name/filename inputs are already escaped via quote(), which leaves boundary() as the one un-validated injection surface. Suggest rejecting any non-RFC-7230-token character in boundary().

File parts omit Content-Type when no media type is givenMultipartBody.kt:175 (Part.file). With the default mediaType = null, the file part's headers carry no Content-Type. RFC 7578 §4.4 recommends file parts include one, defaulting to application/octet-stream when the type is unknown. Most servers tolerate the omission so this is an interop nit, but defaulting unknown file parts to application/octet-stream would be more spec-conformant.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add multipart request body support

1 participant