feat: accept raw/binary request bodies on /post and /anything#163
feat: accept raw/binary request bodies on /post and /anything#163cyfung1031 wants to merge 2 commits into
Conversation
/post and /anything currently reject any request body whose Content-Type isn't application/json (or text/plain) with 415 Unsupported Media Type, since Fastify only registers parsers for those two content types by default. That means image uploads, arbitrary binary payloads, multipart/form-data, or any client sending Content-Type: application/octet-stream can't be exercised against these routes at all, unlike httpbin/httpbun. Adds a shared registerRawBodyFallbackParser() (src/routes/http-methods/raw-body.ts) that registers a "*" catch-all content-type parser, scoped to each route's own plugin encapsulation, so it only fills in the gap for content types nothing else already handles - it doesn't change how application/json (or, on top of jaredwray#161, application/x-www-form-urlencoded) bodies are parsed. The accompanying encodeRequestBody() decides how to represent a raw Buffer in the response: utf8 text for textual content types, base64 otherwise - the same rule src/routes/bins/capture.ts already uses for its request-bin bodies. /post's response schema for `body` is relaxed from a strict `{type: "object"}` to accept any JSON value or the utf8/base64 string produced for non-JSON content, matching the `oneOf: object|string|null` pattern the DELETE route already used. /anything's `data`/`form`/`json` fields are relaxed the same way; `AnythingResponse`'s type is updated to match. One user-visible behavior change worth calling out: a POST to /post or /anything with literally no body/Content-Type now returns 200 with body: {} instead of a strict 400 - since the whole point of this change is that these routes accept a much broader range of bodies now, requiring a well-formed object was no longer the right default. (I caught this as a 500 regression against my first draft via a manual curl check, and added a regression test for it - see the "empty body" test cases in post.test.ts and anything/index.test.ts.) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces raw body fallback parsing to handle request bodies that Fastify's built-in parsers do not support, preventing 415 Unsupported Media Type errors. It registers a catch-all parser for non-JSON/text content types and decodes them as UTF-8 or base64 depending on the content type. The review feedback warns that registering the * content type parser multiple times on the same Fastify instance could cause a FST_ERR_CTP_ALREADY_PRESENT error and crash the application, and suggests checking if the parser is already registered before adding it.
| export function registerRawBodyFallbackParser(fastify: FastifyInstance) { | ||
| fastify.addContentTypeParser( | ||
| "*", | ||
| { parseAs: "buffer" }, | ||
| (_request, body, done) => { | ||
| done(null, body); | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Registering a content type parser for * multiple times on the same Fastify instance (for example, in tests or if encapsulation is disabled/bypassed) will cause Fastify to throw a FST_ERR_CTP_ALREADY_PRESENT error and crash the application on startup. To prevent this, use fastify.hasContentTypeParser('*') to check if the parser is already registered before adding it.
export function registerRawBodyFallbackParser(fastify: FastifyInstance) {
if (fastify.hasContentTypeParser("*")) {
return;
}
fastify.addContentTypeParser(
"*",
{ parseAs: "buffer" },
(_request, body, done) => {
done(null, body);
},
);
}There was a problem hiding this comment.
Not applied. Verified empirically: registering "" twice doesn't throw FST_ERR_CTP_ALREADY_PRESENT (only exact content-type strings do), and hasContentTypeParser("") returns false even right after registering it — the suggested guard couldn't have worked. Posted as a reply comment with the test transcript.
There was a problem hiding this comment.
Code Review
This pull request introduces a fallback raw body parser and request body encoder to gracefully handle non-JSON and binary request bodies on the /anything and /post endpoints, preventing 415 Unsupported Media Type errors. The review feedback suggests enhancing the content-type regex to support structured syntax suffixes (like +json and +xml) and conditionally populating the data, form, and json fields on the /anything endpoint based on the request's Content-Type to prevent data leakage.
| const TEXT_CONTENT_TYPE_REGEX = | ||
| /^(text\/|application\/(json|xml|x-www-form-urlencoded|javascript))/i; |
There was a problem hiding this comment.
The current TEXT_CONTENT_TYPE_REGEX does not match structured syntax suffixes like +json or +xml (e.g., application/ld+json, application/hal+json, image/svg+xml). This causes these common textual content types to be base64-encoded instead of decoded as UTF-8 text. We should update the regex to support these suffixes.
| const TEXT_CONTENT_TYPE_REGEX = | |
| /^(text\/|application\/(json|xml|x-www-form-urlencoded|javascript))/i; | |
| const TEXT_CONTENT_TYPE_REGEX = | |
| /^(text\/|application\/(json|xml|x-www-form-urlencoded|javascript)|.*\+(json|xml))/i; |
There was a problem hiding this comment.
Applied, with a fix 4ca6d56: added a trailing \b boundary the suggested regex lacked (raw-body.ts:9-10), which also fixes a pre-existing loose-match bug (application/jsonp false-matching).
| const handler = async (request: FastifyRequest, reply: FastifyReply) => { | ||
| const body = | ||
| encodeRequestBody(request.body, request.headers["content-type"]) ?? {}; | ||
| const response: AnythingResponse = { | ||
| args: request.query as Record<string, unknown>, | ||
| data: (request.body as Record<string, unknown>) ?? {}, | ||
| data: body, | ||
| // biome-ignore lint/suspicious/noExplicitAny: expected | ||
| files: (request.raw as any)?.files ?? {}, | ||
| form: (request.body as Record<string, unknown>) ?? {}, | ||
| form: body, | ||
| headers: request.headers, | ||
| json: (request.body as Record<string, unknown>) ?? {}, | ||
| json: body, | ||
| method: request.method, | ||
| origin: request.headers.origin ?? request.headers.host ?? "", | ||
| url: request.url, |
There was a problem hiding this comment.
In httpbin (and for general compatibility), the /anything endpoint should only populate the json field if the request body is valid JSON, and the form field if the request is form-encoded (application/x-www-form-urlencoded or multipart/form-data). Currently, setting data, form, and json all to the same body value causes raw binary (base64) or plain text payloads to leak into form and json fields. We can conditionally populate these fields based on the request's Content-Type while still returning {} when no body is present to maintain backward compatibility with existing tests.
const handler = async (request: FastifyRequest, reply: FastifyReply) => {
const contentType = request.headers["content-type"] ?? "";
const isJson = /application\/json/i.test(contentType);
const isForm =
/application\/(x-www-form-urlencoded|form-data)/i.test(contentType) ||
contentType.startsWith("multipart/");
const body = encodeRequestBody(request.body, contentType);
const response: AnythingResponse = {
args: request.query as Record<string, unknown>,
data: isForm ? {} : (body ?? {}),
// biome-ignore lint/suspicious/noExplicitAny: expected
files: (request.raw as any)?.files ?? {},
form: isForm ? (body ?? {}) : {},
headers: request.headers,
json: isJson ? (body ?? {}) : (body === undefined ? {} : null),
method: request.method,
origin: request.headers.origin ?? request.headers.host ?? "",
url: request.url,
};There was a problem hiding this comment.
Applied ( 4ca6d56 ), via isJsonContentType/isFormContentType (raw-body.ts:63-72, wired into anything/index.ts:86-98).
| expect(body.data).toBe(png.toString("base64")); | ||
| expect(body.form).toBe(png.toString("base64")); | ||
| expect(body.json).toBe(png.toString("base64")); |
There was a problem hiding this comment.
Update the test assertions to match the corrected /anything behavior where non-form and non-JSON payloads do not leak into the form and json fields.
| expect(body.data).toBe(png.toString("base64")); | |
| expect(body.form).toBe(png.toString("base64")); | |
| expect(body.json).toBe(png.toString("base64")); | |
| expect(body.data).toBe(png.toString("base64")); | |
| expect(body.form).toEqual({}); | |
| expect(body.json).toBeNull(); |
There was a problem hiding this comment.
Applied (4ca6d56), but with {} instead of the suggested toBeNull() — kept consistent with using {} (not null) for the mismatch case, since null would've broken a pre-existing passing test for bodyless GET.
…raw bodies into /anything's form and json fields Per review feedback on jaredwray#163: - TEXT_CONTENT_TYPE_REGEX now also matches structured syntax suffixes (RFC 6839) like application/ld+json, application/vnd.api+json, and image/svg+xml, so these are decoded as utf8 text instead of needlessly base64-encoded. Added a trailing \b so lookalikes like application/jsonp (textual, but not a real "+json" suffix type, and already loosely matched by the pre-existing regex in src/routes/bins/capture.ts) don't false-positive - verified with a standalone regex script before writing any code. - /anything's `form` and `json` fields no longer echo back whatever was in `data` regardless of Content-Type. A binary or plain-text body no longer shows up under both `form` and `json` alongside `data` - each field is now only populated when the Content-Type actually says so (via new isJsonContentType/isFormContentType helpers), matching how httpbin's own /anything endpoint behaves. I did not apply the reviewer's suggested `fastify.hasContentTypeParser("*")` guard against double-registering the catch-all parser: I verified directly (see below) that registering a "*" content-type parser twice on the same Fastify instance does not throw FST_ERR_CTP_ALREADY_PRESENT (that only happens for exact/specific content-type strings) - the second registration just silently wins as the new fallback. I also confirmed hasContentTypeParser("*") returns false even immediately after registering the wildcard parser, so the suggested guard wouldn't have detected anything anyway. Since both call sites (postRoute, anythingRoute) register the exact same no-op passthrough handler, even a genuine double-registration would be a no-op in practice. Full transcript in the PR comment. I also did not apply the reviewer's suggested `json: null` fallback verbatim - it would have broken the pre-existing, already-passing test asserting `json` is `{}` for a plain bodyless GET (unrelated to this PR). Used `{}` instead of `null` for the "wrong content type" case too, keeping that pre-existing contract intact while still fixing the actual leakage the review flagged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for the review. Two of the three points are addressed: Structured syntax suffixes (
const f = Fastify();
f.addContentTypeParser("*", { parseAs: "buffer" }, (r, b, done) => done(null, "FIRST"));
f.addContentTypeParser("*", { parseAs: "buffer" }, (r, b, done) => done(null, "SECOND"));
// -> no throw. "SECOND" wins as the fallback.
f.hasContentTypeParser("*") // -> false, even right after registering itSo two things: registering |
|
Sorry for using Vibe Coding. In one of my projects (https://github.com/scriptscat/scriptcat/), 
Hope you might consider to merge this PR for the API enhancement. |
2 participants
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? Feature —
/postand/anythingnow accept raw/binary request bodies instead of rejecting anything that isn'tapplication/json. Includes one documented behavior change (see below).Summary
/postand/anythingcurrently reject any request body whoseContent-Typeisn'tapplication/json(ortext/plain) with415 Unsupported Media Type, since Fastify only registers parsers for those two content types by default:That means image uploads, arbitrary binary payloads,
multipart/form-data, or any client sendingContent-Type: application/octet-streamcan't be exercised against these routes at all — unlike httpbin/httpbun, where posting arbitrary bytes to these kinds of endpoints is a common use case (this is the last of the compatibility gaps I found migrating a project's test suite onto mockhttp.org, alongside #160, #161, #162).Fix
Adds a shared
registerRawBodyFallbackParser()(src/routes/http-methods/raw-body.ts) that registers a"*"catch-all content-type parser, scoped to each route's own plugin encapsulation (fastify.register(postRoute)/fastify.register(anythingRoute)), so it only fills in the gap for content types nothing else already handles — it doesn't change howapplication/json(or, on top of #161,application/x-www-form-urlencoded) bodies are parsed.The accompanying
encodeRequestBody()decides how to represent a rawBufferin the response: utf8 text for textual content types, base64 otherwise — the same rulesrc/routes/bins/capture.tsalready uses for its request-bin bodies, so this doesn't introduce a new convention./post's response schema forbodyis relaxed from a strict{type: "object"}to accept any value, matching how theDELETEroute's response schema already usesoneOf: [object, string, null]for the same field./anything'sdata/form/jsonfields are relaxed the same way; theAnythingResponsetype is updated to match.One behavior change worth flagging explicitly: a POST to
/postor/anythingwith no body/Content-Type at all now returns200withbody: {}instead of a strict400— since the whole point of this change is accepting a much broader range of bodies, requiring a well-formed object was no longer the right default. I actually caught this as a500crash regression against my first draft via a manual curl check (fast-json-stringify choking on thebodyfield beingundefinedagainst the response schema'srequired), fixed it, and added a dedicated regression test for it (the "empty body" cases inpost.test.tsandanything/index.test.ts).Test plan
test/routes/http-methods/raw-body.test.tscoveringregisterRawBodyFallbackParserandencodeRequestBodydirectly (binary capture, JSON untouched, utf8 vs base64 selection, empty buffer, non-Buffer passthrough).post.test.tsandanything/index.test.ts.pnpm test(lint + full vitest run with coverage) passes locally: 484 tests, 100% line coverage, no lint warnings.tsx src/index.tsinstance: binary PNG upload, multipart upload, JSON object/array bodies, and the empty-body case all behave as expected; confirmed the pre-existing400became a200/{}deliberately (not the500from my first draft).