Skip to content

Drop request body on cross-origin 307/308 redirects - #923

Closed
benoitc wants to merge 1 commit into
masterfrom
fix/redirect-body-cross-origin
Closed

Drop request body on cross-origin 307/308 redirects#923
benoitc wants to merge 1 commit into
masterfrom
fix/redirect-body-cross-origin

Conversation

@benoitc

@benoitc benoitc commented Aug 9, 2026

Copy link
Copy Markdown
Owner

A 307/308 keeps the method and body, but the body was forwarded to the redirect target regardless of origin; only auth and cookie options were stripped. A POST/PUT/PATCH body still reached a different host and leaked request contents, across the HTTP/1.1, HTTP/2 and HTTP/3 paths. Clear the body when the redirect leaves the original host unless the caller set location_trusted, matching the credential-header stripping.

A 307/308 keeps the method and body, but the body was forwarded to the
redirect target regardless of origin. maybe_strip_auth_on_redirect only
filtered auth/cookie options, so a POST/PUT/PATCH body still reached a
different host, leaking request contents (same in the HTTP/1.1, HTTP/2 and
HTTP/3 paths).

Clear the body when the redirect leaves the original host unless the caller
set location_trusted, matching the credential-header stripping.
@benoitc

benoitc commented Aug 9, 2026

Copy link
Copy Markdown
Owner Author

After checking against RFC 9110, I close this one.

For a 307/308 the client must keep the same method and the body, this is exactly why these codes exist. And the browsers, node-fetch, curl, they all keep the body on a cross-origin redirect too, they only strip the credential headers (Authorization, Cookie, Proxy-Authorization), which we already do. So dropping the body is not correct here, it would break the legit cases like the S3 presigned upload or an API gateway hand-off.

The real risk on cross-origin is the credentials, and this part is already handled. So no need for this change.

@benoitc benoitc closed this Aug 9, 2026
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