Repository navigation
fix(response-headers): make reported Content-Length match the body - #37
Conversation
s3anw3
left a comment
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| Ok(response_builder | ||
| .content_type("application/json") |
There was a problem hiding this comment.
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.
# Conflicts: # src/tests/status.rs
s3anw3
left a comment
There was a problem hiding this comment.
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.
|
Thanks for the review and for merging! Glad the Content-Type change addressed the regression. |
Fixes #34
The JSON body of
/response-headersreported the length of the body before theContent-Lengthkey 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 theContent-Lengthheader 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 --checkandcargo clippypass locally.Thanks!