From ba80ff74a9627f676b4c426587ce5ea487665e46 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Tue, 14 Oct 2008 11:37:59 +0200 Subject: Sanitize the URLs passed to redirect_to to prevent a potential response splitting attack. CGI.rb and mongrel don't do any sanitization of the contents of HTTP headers, so care needs to be taken. --- actionpack/lib/action_controller/response.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb index 54a99996ef..b440065482 100644 --- a/actionpack/lib/action_controller/response.rb +++ b/actionpack/lib/action_controller/response.rb @@ -114,8 +114,8 @@ module ActionController # :nodoc: def redirect(url, status) self.status = status - self.location = url - self.body = "You are being redirected." + self.location = url.gsub(/[\r\n]/, '') + self.body = "You are being redirected." end def sending_file? -- cgit v1.2.3 From fcb45e5ec3c8ab7b11391c0639609918876ddbb2 Mon Sep 17 00:00:00 2001 From: Tim Harper Date: Mon, 20 Oct 2008 13:21:59 -0500 Subject: Ensure ActionView::Helpers::AssetTagHelper::AssetTag::Cache is cleared before loading so changes to asset files are picked up by the broswer [#1233 state:resolved] Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/dispatcher.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 90c8400c11..28f8ce3d53 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -39,7 +39,7 @@ module ActionController # Add a preparation callback. Preparation callbacks are run before every # request in development mode, and before the first request in production # mode. - # + # # An optional identifier may be supplied for the callback. If provided, # to_prepare may be called again with the same identifier to replace the # existing callback. Passing an identifier is a suggested practice if the @@ -144,6 +144,7 @@ module ActionController Routing::Routes.reload ActionController::Base.view_paths.reload! + ActionView::Helpers::AssetTagHelper::AssetTag::Cache.clear end # Cleanup the application by clearing out loaded classes so they can -- cgit v1.2.3 From 9acb88e666269204821b78bec7b72d3d16597096 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 21 Oct 2008 02:30:13 +0200 Subject: Added stale?/fresh? and fresh_when methods to provide a layer of abstraction above request.fresh? and friends [DHH] --- actionpack/lib/action_controller/base.rb | 70 ++++++++++++++++++++++------- actionpack/lib/action_controller/request.rb | 14 +++++- 2 files changed, 65 insertions(+), 19 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 3ede681253..4c5c5ac597 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -965,22 +965,6 @@ module ActionController #:nodoc: render :nothing => true, :status => status end - # Sets the Last-Modified response header. Returns 304 Not Modified if the - # If-Modified-Since request header is <= last modified. - def last_modified!(utc_time) - response.last_modified= utc_time - if request.if_modified_since && request.if_modified_since <= utc_time - head(:not_modified) - end - end - - # Sets the ETag response header. Returns 304 Not Modified if the - # If-None-Match request header matches. - def etag!(etag) - response.etag = etag - head(:not_modified) if response.etag == request.if_none_match - end - # Clears the rendered results, allowing for another render to be performed. def erase_render_results #:nodoc: response.body = nil @@ -1090,6 +1074,54 @@ module ActionController #:nodoc: @performed_redirect = true end + # Sets the etag and/or last_modified on the response and checks it against + # the client request. If the request doesn't match the options provided, the + # request is considered stale and should be generated from scratch. Otherwise, + # it's fresh and we don't need to generate anything and can rely on the default + # reply of "304 Not Modified". + # + # Example: + # + # def show + # @article = Article.find(params[:id]) + # + # if stale?(:etag => @article, :last_modified => @article.created_at.utc) + # @statistics = @article.really_expensive_call + # respond_to do |format| + # # all the supported formats + # end + # end + # end + def stale?(options) + fresh_when(options) + !request.fresh?(response) + end + + # The opposite of stale? provided for parity when that feels more natural. + def fresh?(options) + !stale?(options) + end + + # Sets the etag, last_modified, or both such that the request can be short-circuited + # with a "304 Not Modified" response instead of rendering a template when the request + # is already fresh. + # + # Example: + # + # def show + # @article = Article.find(params[:id]) + # fresh_when(:etag => @article, :last_modified => @article.created_at.utc) + # end + # + # This will render the show template if the request isn't sending a matching etag or + # If-Modified-Since header and just a "304 Not Modified" response if there's a match. + def fresh_when(options) + options.assert_valid_keys(:etag, :last_modified) + + response.etag = options[:etag] if options[:etag] + response.last_modified = options[:last_modified] if options[:last_modified] + end + # Sets a HTTP 1.1 Cache-Control header. Defaults to issuing a "private" instruction, so that # intermediate caches shouldn't cache the response. # @@ -1176,7 +1208,11 @@ module ActionController #:nodoc: end def default_render #:nodoc: - render + if request.fresh?(response) + head :not_modified + else + render + end end def perform_action diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index 5e492e3ee1..9f33cbc55f 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -120,9 +120,19 @@ module ActionController end # Check response freshness (Last-Modified and ETag) against request - # If-Modified-Since and If-None-Match conditions. + # If-Modified-Since and If-None-Match conditions. If both headers are + # supplied, both must match, or the request is not considered fresh. def fresh?(response) - not_modified?(response.last_modified) || etag_matches?(response.etag) + case + when if_modified_since && if_none_match + not_modified?(response.last_modified) && etag_matches?(response.etag) + when if_modified_since + not_modified?(response.last_modified) + when if_none_match + etag_matches?(response.etag) + else + false + end end # Returns the Mime type for the \format used in the request. -- cgit v1.2.3 From 448e7e7c04260c34b816eb951427fc4409843cc8 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 21 Oct 2008 02:54:55 +0200 Subject: Let fresh_when actually do the head(:not_modified). Cleaner and we get the filter halting for free then. --- actionpack/lib/action_controller/base.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 4c5c5ac597..56f4f80617 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1077,8 +1077,7 @@ module ActionController #:nodoc: # Sets the etag and/or last_modified on the response and checks it against # the client request. If the request doesn't match the options provided, the # request is considered stale and should be generated from scratch. Otherwise, - # it's fresh and we don't need to generate anything and can rely on the default - # reply of "304 Not Modified". + # it's fresh and we don't need to generate anything and a reply of "304 Not Modified" is sent. # # Example: # @@ -1102,9 +1101,8 @@ module ActionController #:nodoc: !stale?(options) end - # Sets the etag, last_modified, or both such that the request can be short-circuited - # with a "304 Not Modified" response instead of rendering a template when the request - # is already fresh. + # Sets the etag, last_modified, or both on the response and renders a + # "304 Not Modified" response if the request is already fresh. # # Example: # @@ -1120,6 +1118,10 @@ module ActionController #:nodoc: response.etag = options[:etag] if options[:etag] response.last_modified = options[:last_modified] if options[:last_modified] + + if request.fresh?(response) + head :not_modified + end end # Sets a HTTP 1.1 Cache-Control header. Defaults to issuing a "private" instruction, so that @@ -1208,11 +1210,7 @@ module ActionController #:nodoc: end def default_render #:nodoc: - if request.fresh?(response) - head :not_modified - else - render - end + render end def perform_action -- cgit v1.2.3 From 6fae0a0ec0e00db344369430e4f90e54590a5cb8 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 21 Oct 2008 10:20:30 +0200 Subject: No use case handy for justifying fresh? any more --- actionpack/lib/action_controller/base.rb | 5 ----- 1 file changed, 5 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 56f4f80617..2cff05dfa4 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1096,11 +1096,6 @@ module ActionController #:nodoc: !request.fresh?(response) end - # The opposite of stale? provided for parity when that feels more natural. - def fresh?(options) - !stale?(options) - end - # Sets the etag, last_modified, or both on the response and renders a # "304 Not Modified" response if the request is already fresh. # -- cgit v1.2.3 From a5cdb7a813515fa0cbee23101d2f911b4017ed90 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Tue, 21 Oct 2008 11:34:56 +0100 Subject: Remove stuff that was deprecated in 2-1-stable --- .../lib/action_controller/caching/fragments.rb | 26 ---------------------- 1 file changed, 26 deletions(-) (limited to 'actionpack/lib/action_controller') diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index e9b434dd25..31cbe27452 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -26,32 +26,6 @@ module ActionController #:nodoc: # # expire_fragment(:controller => "topics", :action => "list", :action_suffix => "all_topics") module Fragments - def self.included(base) #:nodoc: - base.class_eval do - class << self - def fragment_cache_store=(store_option) #:nodoc: - ActiveSupport::Deprecation.warn('The fragment_cache_store= method is now use cache_store=') - self.cache_store = store_option - end - - def fragment_cache_store #:nodoc: - ActiveSupport::Deprecation.warn('The fragment_cache_store method is now use cache_store') - cache_store - end - end - - def fragment_cache_store=(store_option) #:nodoc: - ActiveSupport::Deprecation.warn('The fragment_cache_store= method is now use cache_store=') - self.cache_store = store_option - end - - def fragment_cache_store #:nodoc: - ActiveSupport::Deprecation.warn('The fragment_cache_store method is now use cache_store') - cache_store - end - end - end - # Given a key (as described in expire_fragment), returns a key suitable for use in reading, # writing, or expiring a cached fragment. If the key is a hash, the generated key is the return # value of url_for on that hash (without the protocol). All keys are prefixed with "views/" and uses -- cgit v1.2.3