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

Review whether to send request body in redirects #8770

Closed
sbordet opened this issue Oct 26, 2022 · 1 comment · Fixed by #8775
Closed

Review whether to send request body in redirects #8770

sbordet opened this issue Oct 26, 2022 · 1 comment · Fixed by #8775

Comments

@sbordet
Copy link
Contributor

sbordet commented Oct 26, 2022

Jetty version(s)
10+

Enhancement Description
HttpRedirector should probe the request content to know whether it's reproducible like e.g. AuthenticationProtocolHandler does.

@sbordet
Copy link
Contributor Author

sbordet commented Oct 26, 2022

Turns out that we are re-sending the request content even when the redirection changes the method to GET, which is not correct.
The request content should be resent only in case of 307 or 308.

sbordet added a commit that referenced this issue Oct 27, 2022
Now the original request body is re-sent only if the redirect status code is 307 or 308.

In the other cases, it is a redirect to a GET method, so the Location is followed without resending the body.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Nov 8, 2022
Also removing the content headers in case of redirect using GET.

Signed-off-by: Simone Bordet <[email protected]>
sbordet added a commit that referenced this issue Nov 8, 2022
* Fixes #8770 - Review whether to send request body in redirects.

Now the original request body is re-sent only if the redirect status code is 307 or 308.
In the other cases, it is a redirect to a GET method, so the Location is followed without resending the body, and the content headers are removed.

Signed-off-by: Simone Bordet <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant