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

Remove body content from redirect responses #44554

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

jdufresne
Copy link
Contributor

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.

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.
@p8
Copy link
Member

p8 commented Mar 21, 2022

Hey @jdufresne .

Application controllers that wish to add a response body after calling
redirect_to can continue to do so.

Maybe I'm missing something, but how can I set the response body after this change?

@jdufresne
Copy link
Contributor Author

Does the following work for you:

self.response_body = ...

@p8
Copy link
Member

p8 commented Mar 26, 2022

Thanks @jdufresne
Calling after the redirect works for me:

redirect_to(@post)
self.response_body = "Body Content"

carlosantoniodasilva added a commit to heartcombo/devise that referenced this pull request Apr 22, 2022
Rails is no longer returning a message with the response body on
redirects, just an empty body.

rails/rails#44554
carlosantoniodasilva added a commit to heartcombo/devise that referenced this pull request Apr 22, 2022
Rails is no longer returning a message with the response body on
redirects, just an empty body.

rails/rails#44554
varyonic added a commit to varyonic/activeadmin-rails that referenced this pull request Oct 31, 2023
varyonic added a commit to varyonic/activeadmin-rails that referenced this pull request Nov 8, 2023
* rails-7.1:
  Rails 7.1 no longer populates redirect body (rails/rails#44554).
  Calling silence on ActiveSupport::Deprecation is deprecated and will be removed from Rails (use Rails.application.deprecators.silence instead)
  Deprecator setting has been deprecated.
  run db:drop, db:create and db:migrate in a separate commands (probably due to rails/rails#49349)
  Override and revert rails/rails#46699 for now, move test database from /storage back to /db
  Rails 7.1 replaces config.cache_classes with config.enable_reloading in template environment/test.rb
  Add Rails 7.1 test gem file.
  to_default_s is deprecated and will be removed from Rails 7.2 (use to_s instead)
  ActionView::OutputBuffer refactored by rails/rails#45614 (Rails 7.1)
  See rails/rails#36020
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 11, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 12, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 12, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 15, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 16, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 23, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 27, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 29, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 30, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Jan 30, 2024
dombesz pushed a commit to opf/openproject that referenced this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants