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.
rails · Feb 25, 2022 · c2e756a · c2e756a
1 parent a96c5d4
commit c2e756a
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 24 deletions.
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
@@ -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.

diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
@@ -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

diff --git a/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb
@@ -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

diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -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,

diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb
@@ -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
 

diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb
@@ -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
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
@@ -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