diff options
Diffstat (limited to 'actionpack')
18 files changed, 289 insertions, 93 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1b153300b0..0ca3d2eb01 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,7 +1,43 @@ * Add extension synonyms `yml` and `yaml` for MIME type `application/x-yaml`. - + *bogdanvlviv* +* Adds support for including ActionController::Cookies in API controllers. + Previously, including the module would raise when trying to define + a `cookies` helper method. Skip calling #helper_method if it is not + defined -- if we don't have helpers, we needn't define one. + + Fixes #24304 + + *Ryan T. Hosford* + +* ETags: Introduce `Response#strong_etag=` and `#weak_etag=` and analogous + options for `fresh_when` and `stale?`. `Response#etag=` sets a weak ETag. + + Strong ETags are desirable when you're serving byte-for-byte identical + responses that support Range requests, like PDFs or videos (typically + done by reproxying the response from a backend storage service). + Also desirable when fronted by some CDNs that support strong ETags + only, like Akamai. + + *Jeremy Daer* + +* ETags: No longer strips quotes (") from ETag values before comparing them. + Quotes are significant, part of the ETag. A quoted ETag and an unquoted + one are not the same entity. + + *Jeremy Daer* + +* ETags: Support `If-None-Match: *`. Rarely useful for GET requests; meant + to provide some optimistic concurrency control for PUT requests. + + *Jeremy Daer* + +* `ActionDispatch::ParamsParser` is deprecated and was removed from the middleware + stack. To configure the parameter parsers use `ActionDispatch::Request.parameter_parsers=`. + + *tenderlove* + * When a `respond_to` collector with a block doesn't have a response, then a `:no_content` response should be rendered. This brings the default rendering behavior introduced by https://github.com/rails/rails/issues/19036 @@ -179,7 +215,7 @@ *Derek Prior* -* `ActionController::TestCase` will be moved to it's own gem in Rails 5.1 +* `ActionController::TestCase` will be moved to its own gem in Rails 5.1 With the speed improvements made to `ActionDispatch::IntegrationTest` we no longer need to keep two separate code bases for testing controllers. In diff --git a/actionpack/lib/action_controller/api.rb b/actionpack/lib/action_controller/api.rb index ff12705abe..6bbebb7b4c 100644 --- a/actionpack/lib/action_controller/api.rb +++ b/actionpack/lib/action_controller/api.rb @@ -14,22 +14,22 @@ module ActionController # flash, assets, and so on. This makes the entire controller stack thinner, # suitable for API applications. It doesn't mean you won't have such # features if you need them: they're all available for you to include in - # your application, they're just not part of the default API Controller stack. + # your application, they're just not part of the default API controller stack. # - # By default, only the ApplicationController in a \Rails application inherits - # from <tt>ActionController::API</tt>. All other controllers in turn inherit - # from ApplicationController. + # Normally, +ApplicationController+ is the only controller that inherits from + # <tt>ActionController::API</tt>. All other controllers in turn inherit from + # +ApplicationController+. # # A sample controller could look like this: # # class PostsController < ApplicationController # def index - # @posts = Post.all - # render json: @posts + # posts = Post.all + # render json: posts # end # end # - # Request, response and parameters objects all work the exact same way as + # Request, response, and parameters objects all work the exact same way as # <tt>ActionController::Base</tt>. # # == Renders @@ -37,18 +37,18 @@ module ActionController # The default API Controller stack includes all renderers, which means you # can use <tt>render :json</tt> and brothers freely in your controllers. Keep # in mind that templates are not going to be rendered, so you need to ensure - # your controller is calling either <tt>render</tt> or <tt>redirect</tt> in - # all actions, otherwise it will return 204 No Content response. + # your controller is calling either <tt>render</tt> or <tt>redirect_to</tt> in + # all actions, otherwise it will return 204 No Content. # # def show - # @post = Post.find(params[:id]) - # render json: @post + # post = Post.find(params[:id]) + # render json: post # end # # == Redirects # # Redirects are used to move from one action to another. You can use the - # <tt>redirect</tt> method in your controllers in the same way as + # <tt>redirect_to</tt> method in your controllers in the same way as in # <tt>ActionController::Base</tt>. For example: # # def create @@ -56,7 +56,7 @@ module ActionController # # do stuff here # end # - # == Adding new behavior + # == Adding New Behavior # # In some scenarios you may want to add back some functionality provided by # <tt>ActionController::Base</tt> that is not present by default in @@ -72,18 +72,19 @@ module ActionController # # class PostsController < ApplicationController # def index - # @posts = Post.all + # posts = Post.all # # respond_to do |format| - # format.json { render json: @posts } - # format.xml { render xml: @posts } + # format.json { render json: posts } + # format.xml { render xml: posts } # end # end # end # - # Quite straightforward. Make sure to check <tt>ActionController::Base</tt> - # available modules if you want to include any other functionality that is - # not provided by <tt>ActionController::API</tt> out of the box. + # Quite straightforward. Make sure to check the modules included in + # <tt>ActionController::Base</tt> if you want to use any other + # functionality that is not provided by <tt>ActionController::API</tt> + # out of the box. class API < Metal abstract! diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 35befc05e1..480e265e44 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -36,8 +36,23 @@ module ActionController # # === Parameters: # - # * <tt>:etag</tt>. - # * <tt>:last_modified</tt>. + # * <tt>:etag</tt> Sets a "weak" ETag validator on the response. See the + # +:weak_etag+ option. + # * <tt>:weak_etag</tt> Sets a "weak" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A weak ETag indicates semantic + # equivalence, not byte-for-byte equality, so they're good for caching + # HTML pages in browser caches. They can't be used for responses that + # must be byte-identical, like serving Range requests within a PDF file. + # * <tt>:strong_etag</tt> Sets a "strong" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A strong ETag implies exact + # equality: the response must match byte for byte. This is necessary for + # doing Range requests within a large video or PDF file, for example, or + # for compatibility with some CDNs that don't support weak ETags. + # * <tt>:last_modified</tt> Sets a "weak" last-update validator on the + # response. Subsequent requests that set If-Modified-Since may return a + # 304 Not Modified response if last_modified <= If-Modified-Since. # * <tt>:public</tt> By default the Cache-Control header is private, set this to # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current @@ -86,12 +101,16 @@ module ActionController # # before_action { fresh_when @article, template: 'widgets/show' } # - def fresh_when(object = nil, etag: object, last_modified: nil, public: false, template: nil) + def fresh_when(object = nil, etag: nil, weak_etag: nil, strong_etag: nil, last_modified: nil, public: false, template: nil) + weak_etag ||= etag || object unless strong_etag last_modified ||= object.try(:updated_at) || object.try(:maximum, :updated_at) - if etag || template - response.etag = combine_etags(etag: etag, last_modified: last_modified, - public: public, template: template) + if strong_etag + response.strong_etag = combine_etags strong_etag, + last_modified: last_modified, public: public, template: template + elsif weak_etag || template + response.weak_etag = combine_etags weak_etag, + last_modified: last_modified, public: public, template: template end response.last_modified = last_modified if last_modified @@ -107,8 +126,23 @@ module ActionController # # === Parameters: # - # * <tt>:etag</tt>. - # * <tt>:last_modified</tt>. + # * <tt>:etag</tt> Sets a "weak" ETag validator on the response. See the + # +:weak_etag+ option. + # * <tt>:weak_etag</tt> Sets a "weak" ETag validator on the response. + # requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A weak ETag indicates semantic + # equivalence, not byte-for-byte equality, so they're good for caching + # HTML pages in browser caches. They can't be used for responses that + # must be byte-identical, like serving Range requests within a PDF file. + # * <tt>:strong_etag</tt> Sets a "strong" ETag validator on the response. + # Requests that set If-None-Match header may return a 304 Not Modified + # response if it matches the ETag exactly. A strong ETag implies exact + # equality: the response must match byte for byte. This is necessary for + # doing Range requests within a large video or PDF file, for example, or + # for compatibility with some CDNs that don't support weak ETags. + # * <tt>:last_modified</tt> Sets a "weak" last-update validator on the + # response. Subsequent requests that set If-Modified-Since may return a + # 304 Not Modified response if last_modified <= If-Modified-Since. # * <tt>:public</tt> By default the Cache-Control header is private, set this to # +true+ if you want your application to be cacheable by other devices (proxy caches). # * <tt>:template</tt> By default, the template digest for the current @@ -180,8 +214,8 @@ module ActionController # super if stale? @article, template: 'widgets/show' # end # - def stale?(object = nil, etag: object, last_modified: nil, public: nil, template: nil) - fresh_when(object, etag: etag, last_modified: last_modified, public: public, template: template) + def stale?(object = nil, **freshness_kwargs) + fresh_when(object, **freshness_kwargs) !request.fresh?(response) end @@ -231,9 +265,8 @@ module ActionController end private - def combine_etags(options) - etags = etaggers.map { |etagger| instance_exec(options, &etagger) }.compact - etags.unshift options[:etag] + def combine_etags(validator, options) + [validator, *etaggers.map { |etagger| instance_exec(options, &etagger) }].compact end end end diff --git a/actionpack/lib/action_controller/metal/cookies.rb b/actionpack/lib/action_controller/metal/cookies.rb index f8efb2b076..44925641a1 100644 --- a/actionpack/lib/action_controller/metal/cookies.rb +++ b/actionpack/lib/action_controller/metal/cookies.rb @@ -3,7 +3,7 @@ module ActionController #:nodoc: extend ActiveSupport::Concern included do - helper_method :cookies + helper_method :cookies if defined?(helper_method) end private diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 35be6d9300..53527c08b6 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -347,7 +347,12 @@ module ActionController # private # def authenticate # authenticate_or_request_with_http_token do |token, options| - # token == TOKEN + # # Compare the tokens in a time-constant manner, to mitigate + # # timing attacks. + # ActiveSupport::SecurityUtils.secure_compare( + # ::Digest::SHA256.hexdigest(token), + # ::Digest::SHA256.hexdigest(TOKEN) + # ) # end # end # end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index b2f0b382b9..5793e28175 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -213,7 +213,7 @@ module ActionController #:nodoc: if !verified_request? if logger && log_warning_on_csrf_failure - logger.warn "Can't verify CSRF token authenticity" + logger.warn "Can't verify CSRF token authenticity." end handle_unverified_request end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 76e3b4d25a..64672de57e 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -184,6 +184,13 @@ module ActionController # Returns an unsafe, unfiltered # <tt>ActiveSupport::HashWithIndifferentAccess</tt> representation of this # parameter. + # + # params = ActionController::Parameters.new({ + # name: 'Senjougahara Hitagi', + # oddity: 'Heavy stone crab' + # }) + # params.to_unsafe_h + # # => {"name"=>"Senjougahara Hitagi", "oddity" => "Heavy stone crab"} def to_unsafe_h convert_parameters_to_hashes(@parameters, :to_unsafe_h) end diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 4bd727c14e..9fa2e38ae3 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -17,9 +17,7 @@ module ActionDispatch end def if_none_match_etags - (if_none_match ? if_none_match.split(/\s*,\s*/) : []).collect do |etag| - etag.gsub(/^\"|\"$/, "") - end + if_none_match ? if_none_match.split(/\s*,\s*/) : [] end def not_modified?(modified_at) @@ -28,8 +26,8 @@ module ActionDispatch def etag_matches?(etag) if etag - etag = etag.gsub(/^\"|\"$/, "") - if_none_match_etags.include?(etag) + validators = if_none_match_etags + validators.include?(etag) || validators.include?('*') end end @@ -80,27 +78,63 @@ module ActionDispatch set_header DATE, utc_time.httpdate end - # This method allows you to set the ETag for cached content, which - # will be returned to the end user. + # This method sets a weak ETag validator on the response so browsers + # and proxies may cache the response, keyed on the ETag. On subsequent + # requests, the If-None-Match header is set to the cached ETag. If it + # matches the current ETag, we can return a 304 Not Modified response + # with no body, letting the browser or proxy know that their cache is + # current. Big savings in request time and network bandwidth. + # + # Weak ETags are considered to be semantically equivalent but not + # byte-for-byte identical. This is perfect for browser caching of HTML + # pages where we don't care about exact equality, just what the user + # is viewing. # - # By default, Action Dispatch sets all ETags to be weak. - # This ensures that if the content changes only semantically, - # the whole page doesn't have to be regenerated from scratch - # by the web server. With strong ETags, pages are compared - # byte by byte, and are regenerated only if they are not exactly equal. - def etag=(etag) - key = ActiveSupport::Cache.expand_cache_key(etag) - super %(W/"#{Digest::MD5.hexdigest(key)}") + # Strong ETags are considered byte-for-byte identical. They allow a + # browser or proxy cache to support Range requests, useful for paging + # through a PDF file or scrubbing through a video. Some CDNs only + # support strong ETags and will ignore weak ETags entirely. + # + # Weak ETags are what we almost always need, so they're the default. + # Check out `#strong_etag=` to provide a strong ETag validator. + def etag=(weak_validators) + self.weak_etag = weak_validators + end + + def weak_etag=(weak_validators) + set_header 'ETag', generate_weak_etag(weak_validators) + end + + def strong_etag=(strong_validators) + set_header 'ETag', generate_strong_etag(strong_validators) end def etag?; etag; end + # True if an ETag is set and it's a weak validator (preceded with W/) + def weak_etag? + etag? && etag.starts_with?('W/"') + end + + # True if an ETag is set and it isn't a weak validator (not preceded with W/) + def strong_etag? + etag? && !weak_etag? + end + private DATE = 'Date'.freeze LAST_MODIFIED = "Last-Modified".freeze SPECIAL_KEYS = Set.new(%w[extras no-cache max-age public private must-revalidate]) + def generate_weak_etag(validators) + "W/#{generate_strong_etag(validators)}" + end + + def generate_strong_etag(validators) + %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(validators))}") + end + def cache_control_segments if cache_control = _cache_control cache_control.delete(' ').split(',') diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 5c3b7245d6..69a934b7cd 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -5,7 +5,7 @@ module ActionDispatch # env = { "CONTENT_TYPE" => "text/plain", "HTTP_USER_AGENT" => "curl/7.43.0" } # headers = ActionDispatch::Http::Headers.new(env) # headers["Content-Type"] # => "text/plain" - # headers["User-Agent"] # => "curl/7/43/0" + # headers["User-Agent"] # => "curl/7.43.0" # # Also note that when headers are mapped to CGI-like variables by the Rack # server, both dashes and underscores are converted to underscores. This diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 5841c978af..faf3262b8f 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -37,6 +37,7 @@ module ActionDispatch # The +parsers+ argument can take Hash of parsers where key is identifying # content mime type, and value is a lambda that is going to process data. def self.new(app, parsers = {}) + ActiveSupport::Deprecation.warn('ActionDispatch::ParamsParser is deprecated and will be removed in Rails 5.1. Configure the parameter parsing in ActionDispatch::Request.parameter_parsers.') parsers = parsers.transform_keys { |key| key.respond_to?(:symbol) ? key.symbol : key } ActionDispatch::Request.parameter_parsers = ActionDispatch::Request::DEFAULT_PARSERS.merge(parsers) app diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 16b430c36e..5a747b5f17 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1598,7 +1598,7 @@ module ActionDispatch route_options = options.dup if _path && option_path ActiveSupport::Deprecation.warn <<-eowarn -Specifying strings for both :path and the route path is deprecated. Change things like this: +Specifying strings for both :path and the route path is deprecated. Change things like this: match #{_path.inspect}, :path => #{option_path.inspect} diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 85f202b823..16237bd564 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -517,14 +517,14 @@ module ActionDispatch if route.segment_keys.include?(:controller) ActiveSupport::Deprecation.warn(<<-MSG.squish) Using a dynamic :controller segment in a route is deprecated and - will be removed in Rails 5.1 + will be removed in Rails 5.1. MSG end if route.segment_keys.include?(:action) ActiveSupport::Deprecation.warn(<<-MSG.squish) Using a dynamic :action segment in a route is deprecated and - will be removed in Rails 5.1 + will be removed in Rails 5.1. MSG end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 60c562d7cd..69ae5a8468 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -95,7 +95,7 @@ module ActionDispatch ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc) xhr and xml_http_request methods are deprecated in favor of - `get "/posts", xhr: true` and `post "/posts/1", xhr: true` + `get "/posts", xhr: true` and `post "/posts/1", xhr: true`. MSG process(request_method, path, params: params, headers: headers, xhr: true) diff --git a/actionpack/test/controller/api/with_cookies_test.rb b/actionpack/test/controller/api/with_cookies_test.rb new file mode 100644 index 0000000000..4491dc9002 --- /dev/null +++ b/actionpack/test/controller/api/with_cookies_test.rb @@ -0,0 +1,21 @@ +require 'abstract_unit' + +class WithCookiesController < ActionController::API + include ActionController::Cookies + + def with_cookies + render plain: cookies[:foobar] + end +end + +class WithCookiesTest < ActionController::TestCase + tests WithCookiesController + + def test_with_cookies + request.cookies[:foobar] = 'bazbang' + + get :with_cookies + + assert_equal 'bazbang', response.body + end +end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 0c3884cd38..a7759c080b 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -205,7 +205,7 @@ module ActionController def overfill_buffer_and_die logger = ActionController::Base.logger || Logger.new($stdout) response.stream.on_error do - logger.warn 'Error while streaming' + logger.warn 'Error while streaming.' error_latch.count_down end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 82fc8b0f8a..f42efd35af 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -16,6 +16,10 @@ class TestControllerWithExtraEtags < ActionController::Base render plain: "stale" if stale?(etag: %w(1 2 3), template: false) end + def strong + render plain: "stale" if stale?(strong_etag: 'strong') + end + def with_template if stale? template: 'test/hello_world' render plain: 'stale' @@ -385,7 +389,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified_but_etag_differs @request.if_modified_since = @last_modified - @request.if_none_match = "234" + @request.if_none_match = '"234"' get :conditional_hello assert_response :success end @@ -414,7 +418,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified_but_etag_differs_with_record @request.if_modified_since = @last_modified - @request.if_none_match = "234" + @request.if_none_match = '"234"' get :conditional_hello_with_record assert_response :success end @@ -442,7 +446,7 @@ class LastModifiedRenderTest < ActionController::TestCase def test_request_not_modified_but_etag_differs_with_collection_of_records @request.if_modified_since = @last_modified - @request.if_none_match = "234" + @request.if_none_match = '"234"' get :conditional_hello_with_collection_of_records assert_response :success end @@ -477,8 +481,26 @@ end class EtagRenderTest < ActionController::TestCase tests TestControllerWithExtraEtags + def test_strong_etag + @request.if_none_match = strong_etag(['strong', 'ab', :cde, [:f]]) + get :strong + assert_response :not_modified + + @request.if_none_match = '*' + get :strong + assert_response :not_modified + + @request.if_none_match = '"strong"' + get :strong + assert_response :ok + + @request.if_none_match = weak_etag(['strong', 'ab', :cde, [:f]]) + get :strong + assert_response :ok + end + def test_multiple_etags - @request.if_none_match = etag(["123", 'ab', :cde, [:f]]) + @request.if_none_match = weak_etag(["123", 'ab', :cde, [:f]]) get :fresh assert_response :not_modified @@ -488,7 +510,7 @@ class EtagRenderTest < ActionController::TestCase end def test_array - @request.if_none_match = etag([%w(1 2 3), 'ab', :cde, [:f]]) + @request.if_none_match = weak_etag([%w(1 2 3), 'ab', :cde, [:f]]) get :array assert_response :not_modified @@ -523,9 +545,14 @@ class EtagRenderTest < ActionController::TestCase end end - def etag(record) - %(W/"#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") - end + private + def weak_etag(record) + "W/#{strong_etag record}" + end + + def strong_etag(record) + %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}") + end end class MetalRenderTest < ActionController::TestCase @@ -713,20 +740,24 @@ class HttpCacheForeverTest < ActionController::TestCase def test_cache_with_public get :cache_me_forever, params: {public: true} + assert_response :ok assert_equal "max-age=#{100.years}, public", @response.headers["Cache-Control"] assert_not_nil @response.etag + assert @response.weak_etag? end def test_cache_with_private get :cache_me_forever + assert_response :ok assert_equal "max-age=#{100.years}, private", @response.headers["Cache-Control"] assert_not_nil @response.etag - assert_response :success + assert @response.weak_etag? end def test_cache_response_code_with_if_modified_since get :cache_me_forever - assert_response :success + assert_response :ok + @request.if_modified_since = @response.headers['Last-Modified'] get :cache_me_forever assert_response :not_modified @@ -734,13 +765,10 @@ class HttpCacheForeverTest < ActionController::TestCase def test_cache_response_code_with_etag get :cache_me_forever - assert_response :success - @request.if_modified_since = @response.headers['Last-Modified'] - @request.if_none_match = @response.etag + assert_response :ok + @request.if_none_match = @response.etag get :cache_me_forever assert_response :not_modified - @request.if_modified_since = @response.headers['Last-Modified'] - @request.if_none_match = @response.etag end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 0edad72fd9..a4cb8ce449 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -1152,36 +1152,41 @@ class RequestParameterFilter < BaseRequestTest end class RequestEtag < BaseRequestTest - test "if_none_match_etags none" do + test "always matches *" do + request = stub_request('HTTP_IF_NONE_MATCH' => '*') + + assert_equal '*', request.if_none_match + assert_equal ['*'], request.if_none_match_etags + + assert request.etag_matches?('"strong"') + assert request.etag_matches?('W/"weak"') + assert_not request.etag_matches?(nil) + end + + test "doesn't match absent If-None-Match" do request = stub_request assert_equal nil, request.if_none_match assert_equal [], request.if_none_match_etags - assert !request.etag_matches?("foo") - assert !request.etag_matches?(nil) - end - test "if_none_match_etags single" do - header = 'the-etag' - request = stub_request('HTTP_IF_NONE_MATCH' => header) - - assert_equal header, request.if_none_match - assert_equal [header], request.if_none_match_etags - assert request.etag_matches?("the-etag") + assert_not request.etag_matches?("foo") + assert_not request.etag_matches?(nil) end - test "if_none_match_etags quoted single" do + test "matches opaque ETag validators without unquoting" do header = '"the-etag"' request = stub_request('HTTP_IF_NONE_MATCH' => header) assert_equal header, request.if_none_match - assert_equal ['the-etag'], request.if_none_match_etags - assert request.etag_matches?("the-etag") + assert_equal ['"the-etag"'], request.if_none_match_etags + + assert request.etag_matches?('"the-etag"') + assert_not request.etag_matches?("the-etag") end test "if_none_match_etags multiple" do header = 'etag1, etag2, "third etag", "etag4"' - expected = ['etag1', 'etag2', 'third etag', 'etag4'] + expected = ['etag1', 'etag2', '"third etag"', '"etag4"'] request = stub_request('HTTP_IF_NONE_MATCH' => header) assert_equal header, request.if_none_match diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index cd385982d9..658e0d004b 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -189,7 +189,7 @@ class ResponseTest < ActiveSupport::TestCase assert_equal({"user_name" => "david", "login" => nil}, @response.cookies) end - test "read cache control" do + test "read ETag and Cache-Control" do resp = ActionDispatch::Response.new.tap { |response| response.cache_control[:public] = true response.etag = '123' @@ -197,6 +197,9 @@ class ResponseTest < ActiveSupport::TestCase } resp.to_a + assert resp.etag? + assert resp.weak_etag? + assert_not resp.strong_etag? assert_equal('W/"202cb962ac59075b964b07152d234b70"', resp.etag) assert_equal({:public => true}, resp.cache_control) @@ -204,6 +207,20 @@ class ResponseTest < ActiveSupport::TestCase assert_equal('W/"202cb962ac59075b964b07152d234b70"', resp.headers['ETag']) end + test "read strong ETag" do + resp = ActionDispatch::Response.new.tap { |response| + response.cache_control[:public] = true + response.strong_etag = '123' + response.body = 'Hello' + } + resp.to_a + + assert resp.etag? + assert_not resp.weak_etag? + assert resp.strong_etag? + assert_equal('"202cb962ac59075b964b07152d234b70"', resp.etag) + end + test "read charset and content type" do resp = ActionDispatch::Response.new.tap { |response| response.charset = 'utf-16' @@ -446,11 +463,19 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest assert_equal('application/xml; charset=utf-16', @response.headers['Content-Type']) end - test "we can set strong ETag by directly adding it as header" do - @response = ActionDispatch::Response.create - @response.add_header "ETag", '"202cb962ac59075b964b07152d234b70"' + test "strong ETag validator" do + @app = lambda { |env| + ActionDispatch::Response.new.tap { |resp| + resp.strong_etag = '123' + resp.body = 'Hello' + resp.request = ActionDispatch::Request.empty + }.to_a + } + + get '/' + assert_response :ok - assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) assert_equal('"202cb962ac59075b964b07152d234b70"', @response.headers['ETag']) + assert_equal('"202cb962ac59075b964b07152d234b70"', @response.etag) end end |