Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(HttpResponse): skip setting "Content-Length" if it is already set #2228

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

kettanaito
Copy link
Member

@kettanaito kettanaito commented Aug 1, 2024

When upgrading @mswjs/interceptors to 0.33 (in #2011), I've noticed a failing mocked binary response test: it was listing the Content-Length header value twice

Content-Length: 1044, 1044

Warning

This indicates a deeper issue, as .set() on Headers instance must not join values. Something goes off here. Raw Headers from Undici behaved correctly on that branch.

We don't have any check if the Content-Length response header has already been set on HttpResponse.arrayBuffer(), which means MSW will disregard your custom Content-Length, which is a bug.

@@ -137,7 +137,7 @@ export class HttpResponse extends Response {
static arrayBuffer(body?: ArrayBuffer, init?: HttpResponseInit): Response {
const responseInit = normalizeResponseInit(init)

if (body) {
if (body && !responseInit.headers.has('Content-Length')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added the missing check.

@kettanaito kettanaito merged commit a0234c9 into main Aug 1, 2024
12 checks passed
@kettanaito kettanaito deleted the fix/arraybuffer-content-length branch August 1, 2024 12:25
@kettanaito
Copy link
Member Author

Released: v2.3.5 🎉

This has been released in v2.3.5!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

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.

1 participant