Remove body content from redirect responses
Modern browsers don't render this HTML so it goes unused in practice. The delivered bytes are therefore a small waste (although very small) and unnecessary and could be optimized away. Additionally, the HTML fails validation. Using the W3C v.Nu, we see the following errors: Warning: Consider adding a lang attribute to the html start tag to declare the language of this document. Error: Start tag seen without seeing a doctype first. Expected <!DOCTYPE html>. Error: Element head is missing a required instance of child element title. These errors may surface in site-wide compliance tests (either internal tests or external contractual tests). Avoid the false positives by removing the HTML. While these warnings and errors could be resolved, it would be simpler on future maintenance to remove the body altogether (especially as it isn't rendered by the browser). As the same string is copied around a few places, this removes multiple touch points to resolve the current validation errors as well as new ones. Many other frameworks and web servers don't include an HTML body on redirect, so there isn't a reason for Rails to do so. By removing the custom Rails HTML, there are fewing "fingerprints" that a malicious bot could use to identify the backend technologies. Application controllers that wish to add a response body after calling redirect_to can continue to do so.
This commit is contained in:
parent
a96c5d4533
commit
c2e756a944
@ -1,3 +1,10 @@
|
||||
* Make `redirect_to` return an empty response body.
|
||||
|
||||
Application controllers that wish to add a response body after calling
|
||||
`redirect_to` can continue to do so.
|
||||
|
||||
*Jon Dufresne*
|
||||
|
||||
* Use non-capturing group for subdomain matching in `ActionDispatch::HostAuthorization`
|
||||
|
||||
Since we do nothing with the captured subdomain group, we can use a non-capturing group instead.
|
||||
|
@ -87,7 +87,7 @@ def redirect_to(options = {}, response_options = {})
|
||||
|
||||
self.status = _extract_redirect_to_status(options, response_options)
|
||||
self.location = _enforce_open_redirect_protection(_compute_redirect_to_location(request, options), allow_other_host: allow_other_host)
|
||||
self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>"
|
||||
self.response_body = ""
|
||||
end
|
||||
|
||||
# Soft deprecated alias for #redirect_back_or_to where the +fallback_location+ location is supplied as a keyword argument instead
|
||||
|
@ -30,7 +30,7 @@ def redirect_to(location)
|
||||
uri = URI.parse location
|
||||
|
||||
if uri.relative? || uri.scheme == "http" || uri.scheme == "https"
|
||||
body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>"
|
||||
body = ""
|
||||
else
|
||||
return [400, { "Content-Type" => "text/plain" }, ["Invalid redirection URI"]]
|
||||
end
|
||||
|
@ -38,7 +38,7 @@ def serve(req)
|
||||
|
||||
req.commit_flash
|
||||
|
||||
body = %(<html><body>You are being <a href="#{ERB::Util.unwrapped_html_escape(uri.to_s)}">redirected</a>.</body></html>)
|
||||
body = ""
|
||||
|
||||
headers = {
|
||||
"Location" => uri.to_s,
|
||||
|
@ -349,7 +349,7 @@ def test_redirect
|
||||
assert_response 302
|
||||
assert_response :redirect
|
||||
assert_response :found
|
||||
assert_equal "<html><body>You are being <a href=\"http://www.example.com/get\">redirected</a>.</body></html>", response.body
|
||||
assert_equal "", response.body
|
||||
assert_kind_of Nokogiri::HTML::Document, html_document
|
||||
assert_equal 1, request_count
|
||||
|
||||
|
@ -331,11 +331,7 @@ def setup
|
||||
def verify_redirect(url, status = 301)
|
||||
assert_equal status, response.status
|
||||
assert_equal url, response.headers["Location"]
|
||||
assert_equal expected_redirect_body(url), response.body
|
||||
end
|
||||
|
||||
def expected_redirect_body(url)
|
||||
%(<html><body>You are being <a href="#{url}">redirected</a>.</body></html>)
|
||||
assert_equal "", response.body
|
||||
end
|
||||
end
|
||||
|
||||
@ -460,11 +456,7 @@ def app
|
||||
def verify_redirect(url, status = 301)
|
||||
assert_equal status, response.status
|
||||
assert_equal url, response.headers["Location"]
|
||||
assert_equal expected_redirect_body(url), response.body
|
||||
end
|
||||
|
||||
def expected_redirect_body(url)
|
||||
%(<html><body>You are being <a href="#{url}">redirected</a>.</body></html>)
|
||||
assert_equal "", response.body
|
||||
end
|
||||
end
|
||||
end
|
||||
|
@ -3914,11 +3914,7 @@ def with_https
|
||||
def verify_redirect(url, status = 301)
|
||||
assert_equal status, @response.status
|
||||
assert_equal url, @response.headers["Location"]
|
||||
assert_equal expected_redirect_body(url), @response.body
|
||||
end
|
||||
|
||||
def expected_redirect_body(url)
|
||||
%(<html><body>You are being <a href="#{ERB::Util.h(url)}">redirected</a>.</body></html>)
|
||||
assert_equal "", @response.body
|
||||
end
|
||||
end
|
||||
|
||||
@ -4341,11 +4337,7 @@ def app; APP end
|
||||
def verify_redirect(url, status = 301)
|
||||
assert_equal status, @response.status
|
||||
assert_equal url, @response.headers["Location"]
|
||||
assert_equal expected_redirect_body(url), @response.body
|
||||
end
|
||||
|
||||
def expected_redirect_body(url)
|
||||
%(<html><body>You are being <a href="#{ERB::Util.h(url)}">redirected</a>.</body></html>)
|
||||
assert_equal "", @response.body
|
||||
end
|
||||
end
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user