Skip to content

fix(response-headers): make reported Content-Length match the body - #37

Merged
s3anw3 merged 3 commits into
seedvector:mainfrom
OpenDeernet:fix-response-headers-content-length
Nov 9, 2026
Merged

s3anw3 merged 3 commits into
seedvector:mainfrom
OpenDeernet:fix-response-headers-content-length

Conversation

@OpenDeernet

Copy link
Copy Markdown
Contributor

Fixes #34

The JSON body of /response-headers reported the length of the body before the Content-Length key was added (e.g. 35 when the real body is 57 bytes). The handler now re-serializes until the reported value matches the body size, like httpbin does. That usually takes two rounds, and the old iteration cap stays as a safety net. The body is sent as the exact string that was measured, so the Content-Length header and the JSON value agree.

Query params are still reflected as headers, and the ?body= path is unchanged.

Added a test that checks the JSON value against the real body length and the header, with and without query params. cargo test, cargo fmt --check and cargo clippy pass locally.

Thanks!

@s3anw3 s3anw3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for fixing this! The iterative length calculation looks good, and all 274 library tests passed locally. I found one Content-Type regression that needs to be addressed before merging; details are inline. An additional regression test fails on this PR and passes with the original response-headers handler.

Comment thread src/handlers/response_inspection.rs Outdated
}

Ok(response_builder
.content_type("application/json")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you preserve a caller-supplied Content-Type here? For example, /response-headers?Content-Type=text/plain previously returned a text/plain response header, but now returns application/json. The previous .json() call only supplied a default when the header was absent; .content_type() unconditionally replaces the header appended from the query parameters.

Please set the JSON default only when Content-Type is absent (treating header names case-insensitively), and add regression coverage for GET and POST. The tests should verify that a supplied Content-Type is preserved, while requests without an override still default to application/json and the reported Content-Length continues to match the serialized body.

@s3anw3 s3anw3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for addressing the feedback! The update preserves caller-supplied Content-Type headers case-insensitively, and the GET/POST regression tests cover overrides, the JSON default, and the reported body length. My previous concern is resolved. All 277 local tests and the formatting check pass, and GitHub CI is green. No blocking findings.

@s3anw3
s3anw3 merged commit 813621b into seedvector:main Nov 9, 2026
1 check passed
@OpenDeernet

Copy link
Copy Markdown
Contributor Author

Thanks for the review and for merging! Glad the Content-Type change addressed the regression.

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.

/response-headers: Content-Length in the JSON body doesn't match the actual body length

2 participants