diff options
author | David Heinemeier Hansson <david@loudthinking.com> | 2016-08-22 13:34:35 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-08-22 13:34:35 -0700 |
commit | debd774d632ae7e1e6c0a7d0306979159df39b63 (patch) | |
tree | 743d98e205e426284db8b823ad4cd133188c96e0 | |
parent | 3bfd35245278f410bb2165e859f1cfeb352e0d77 (diff) | |
download | rails-debd774d632ae7e1e6c0a7d0306979159df39b63.tar.gz rails-debd774d632ae7e1e6c0a7d0306979159df39b63.tar.bz2 rails-debd774d632ae7e1e6c0a7d0306979159df39b63.zip |
Include the content of the flash in the auto-generated etag (#26250)
Include the content of the flash in the auto-generated etag
-rw-r--r-- | actionpack/CHANGELOG.md | 12 | ||||
-rw-r--r-- | actionpack/lib/action_controller.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_controller/base.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/etag_with_flash.rb | 16 | ||||
-rw-r--r-- | actionpack/test/controller/flash_test.rb | 29 |
5 files changed, 59 insertions, 0 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d1a2b9b827..a66a1e8af3 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,15 @@ +* Include the content of the flash in the auto-generated etag. This solves the following problem: + + 1. POST /messages + 2. redirect_to messages_url, notice: 'Message was created' + 3. GET /messages/1 + 4. GET /messages + + Step 4 would before still include the flash message, even though it's no longer relevant, + because the etag cache was recorded with the flash in place and didn't change when it was gone. + + *DHH* + * SSL: Changes redirect behavior for all non-GET and non-HEAD requests (like POST/PUT/PATCH etc) to `http://` resources to redirect to `https://` with a [307 status code](http://tools.ietf.org/html/rfc7231#section-6.4.7) instead of [301 status code](http://tools.ietf.org/html/rfc7231#section-6.4.2). diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index fc86a907b3..50f20aa789 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -23,6 +23,7 @@ module ActionController autoload :Cookies autoload :DataStreaming autoload :EtagWithTemplateDigest + autoload :EtagWithFlash autoload :Flash autoload :ForceSSL autoload :Head diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 68a526eccb..ca8066cd82 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -213,6 +213,7 @@ module ActionController Renderers::All, ConditionalGet, EtagWithTemplateDigest, + EtagWithFlash, Caching, MimeResponds, ImplicitRender, diff --git a/actionpack/lib/action_controller/metal/etag_with_flash.rb b/actionpack/lib/action_controller/metal/etag_with_flash.rb new file mode 100644 index 0000000000..474d75f02e --- /dev/null +++ b/actionpack/lib/action_controller/metal/etag_with_flash.rb @@ -0,0 +1,16 @@ +module ActionController + # When you're using the flash, it's generally used as a conditional on the view. + # This means the content of the view depends on the flash. Which in turn means + # that the etag for a response should be computed with the content of the flash + # in mind. This does that by including the content of the flash as a component + # in the etag that's generated for a response. + module EtagWithFlash + extend ActiveSupport::Concern + + include ActionController::ConditionalGet + + included do + etag { flash unless flash.empty? } + end + end +end diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb index cabbe2d608..e5f24f1a3a 100644 --- a/actionpack/test/controller/flash_test.rb +++ b/actionpack/test/controller/flash_test.rb @@ -263,6 +263,13 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest flash[:bar] = "for great justice" head :ok end + + def set_flash_optionally + flash.now.notice = params[:flash] + if stale? etag: "abe" + render inline: "maybe flash" + end + end end def test_flash @@ -310,6 +317,28 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest end end + def test_flash_factored_into_etag + with_test_route_set do + get "/set_flash_optionally" + no_flash_etag = response.etag + + get "/set_flash_optionally", params: { flash: "hello!" } + hello_flash_etag = response.etag + + assert_not_equal no_flash_etag, hello_flash_etag + + get "/set_flash_optionally", params: { flash: "hello!" } + another_hello_flash_etag = response.etag + + assert_equal another_hello_flash_etag, hello_flash_etag + + get "/set_flash_optionally", params: { flash: "goodbye!" } + goodbye_flash_etag = response.etag + + assert_not_equal another_hello_flash_etag, goodbye_flash_etag + end + end + private # Overwrite get to send SessionSecret in env hash |