Skip to content

fix: don't write a response body for null body status codes - #342

Merged
naorpeled merged 1 commit into
jeremydaly:mainfrom
naorpeled:fix/null-body-status-codes-in-send
Aug 8, 2026
Merged

fix: don't write a response body for null body status codes#342
naorpeled merged 1 commit into
jeremydaly:mainfrom
naorpeled:fix/null-body-status-codes-in-send

Conversation

@naorpeled

Copy link
Copy Markdown
Collaborator

Follow-up to #336, which fixed sendStatus() for RFC 9110 null body status codes. The same leak is reachable through every other response method:

res.status(204).json({ a: 1 })   // body: '{"a":1}'
res.status(205).send('hi')       // body: 'hi'
res.status(304).html('<p>hi</p>')// body: '<p>hi</p>'

Change

Adds UTILS.isNullBodyStatus() and applies it once in RESPONSE.send(), so send/json/jsonp/html/sendFile/error all inherit the behavior instead of each needing its own special case. Express takes the same approach for the same reason (express/lib/response.js).

Notes for review

  • Overlaps with Don't write a response body for null body status codes in sendStatus #336. This also fixes sendStatus(), so once both land statusBodyLookup is redundant and sendStatus can go back to statusLookup. Happy to rebase and fold that in after Don't write a response body for null body status codes in sendStatus #336 merges — I've left Don't write a response body for null body status codes in sendStatus #336 alone so the original reporter's fix lands on its own.
  • This discards user-supplied bodies. That's the substantive decision here, and it's a wider behavior change than Don't write a response body for null body status codes in sendStatus #336: res.status(204).json({...}) will silently drop the payload. Express does the same, but for a reason that doesn't transfer — Node's http layer physically can't write a body for 204/304, so Express is aligning headers with a reality it can't change. lambda-api is just building a JSON object, so this is a deliberate choice to enforce the spec rather than a constraint. Flagging it explicitly in case you'd rather pass the body through and let callers own the violation.
  • Headers are left alone. Express also strips Content-Type/Content-Length/Transfer-Encoding for these codes. I didn't, to keep the diff to body semantics and because existing 304 ETag responses already include content-type (__tests__/etag.unit.js). Easy to add if you want the full Express behavior.
  • 1xx is unreachable through API Gateway/ALB proxy integrations, but included for correctness since it costs nothing.

Testing

  • New integration tests in __tests__/responses.unit.js for json(), send(), html(), and sendStatus() on null body codes, plus a 200 control case.
  • Table-driven unit tests for isNullBodyStatus covering boundaries (199/200, 203/204/205/206, 303/304/305), string inputs, and null/undefined.
  • Full suite passes: 27 suites, 510 passed / 4 skipped. No existing test relied on a body for these codes.

Docs

Updated the status(code) section of the README — its example was res.status(304).send('Not Modified'), which this change makes misleading.

Status codes that must not carry content per RFC 9110 (1xx, 204, 205,
304) were still emitting a body when set via status() and sent through
send(), json(), html(), etc.

Moves the check into send() so every response method inherits it,
rather than special-casing sendStatus() alone.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Broadens #336 by enforcing bodyless HTTP statuses across all response methods.

Changes:

  • Centralizes null-body status detection in send().
  • Adds utility and integration tests.
  • Updates status documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/lib/utils.js Adds null-body status detection.
src/lib/response.js Discards prohibited response bodies.
README.md Documents bodyless statuses.
__tests__/utils.unit.js Tests status detection boundaries.
__tests__/responses.unit.js Tests response methods and control behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/lib/utils.js
@naorpeled
naorpeled force-pushed the fix/null-body-status-codes-in-send branch from dc4becd to 7e7d096 Compare August 8, 2026 16:16
@naorpeled
naorpeled merged commit 9a031dd into jeremydaly:main Aug 8, 2026
8 checks passed
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.

2 participants