rails
rails copied to clipboard
Add `redirect_code_for_unsafe_http_methods` config
When a client follows an HTTP 302 redirect, it will typically use the same HTTP method as the original request. This can cause issues when, for example, redirecting an XHR DELETE
request after a destroy
. (Note that many clients make an exception for POST
, and will use GET
to follow a 302 redirect in that case.)
The solution so far has been for users to specify an explicit response code in such cases. For example:
def destroy
Post.destroy(params[:id])
redirect_to root_path, status: 303
end
This commit adds a redirect_code_for_unsafe_http_methods
configuration setting that allows users to specify a default HTTP response code to use when redirecting a request made with an unsafe HTTP method, such as POST
or DELETE
. For example, when set to 303, the explicit response code may be omitted:
def destroy
Post.destroy(params[:id])
redirect_to root_path
end
Note that many clients make an exception for POST, and will use GET to follow a 302 redirect in that case.
I think this is probably worth specific discussion.
The version of this change that I had pictured after seeing hotwired/turbo#84 (via #45383) had us retaining the legacy 302 status for "legacy HTTP actions", rather than safe/idempotent ones... which leaves us joining in the codification of existing non-conforming practice, but seems like it gives a decent reduction in potential surprise?
The version of this change that I had pictured after seeing hotwired/turbo#84 (via #45383) had us retaining the legacy 302 status for "legacy HTTP actions", rather than safe/idempotent ones... which leaves us joining in the codification of existing non-conforming practice, but seems like it gives a decent reduction in potential surprise?
One potential issue with excluding POST
is that form helper forms always use POST
. The intended HTTP method is then exposed via request_method
. But since we are using method
(which I believe is correct), a submitted form would get 302, but an API call or controller test would get 303.
I'd love to see this PR brought back to life. As part of rolling out Turbo Drive across a large app we are looking to monkey patch ApplicationController
def redirect_to(options = {}, response_options = {})
if options.is_a?(Hash)
options[:status] = :see_other
else
response_options[:status] = :see_other
end
super(options, response_options)
end
But given it's basically a requirement for using Hotwire properly, this feels like something Rails should support out of the box.
That said, if making it a default is controversial, could we just add the config and consider the default in a separate PR?