Permalink
Browse files

Escape the extension when normalizing the action cache path.

Although no recognized formats use non-ASCII characters, sometimes they
can be included in the :format parameter because of invalid URLS. To
prevent encoding incompatibility errors we need to escape them before
passing the path to URI.unescape.

Closes #4379
  • Loading branch information...
pixeltrix committed May 20, 2012
1 parent 3fc561a commit 79b38c386ae3817df5321522ab9ea169c028de96
Showing with 33 additions and 3 deletions.
  1. +3 −2 actionpack/lib/action_controller/caching/actions.rb
  2. +30 −1 actionpack/test/controller/caching_test.rb
@@ -47,7 +47,7 @@ module Caching
# And you can also use <tt>:if</tt> (or <tt>:unless</tt>) to pass a
# proc that specifies when the action should be cached.
#
- # As of Rails 3.0, you can also pass <tt>:expires_in</tt> with a time
+ # As of Rails 3.0, you can also pass <tt>:expires_in</tt> with a time
# interval (in seconds) to schedule expiration of the cached item.
#
# The following example depicts some of the points made above:
@@ -178,8 +178,9 @@ def initialize(controller, options = {}, infer_extension = true)
private
def normalize!(path)
+ ext = URI.parser.escape(extension) if extension
path << 'index' if path[-1] == ?/
- path << ".#{extension}" if extension and !path.split('?', 2).first.ends_with?(".#{extension}")
+ path << ".#{ext}" if extension and !path.split('?', 2).first.ends_with?(".#{ext}")
URI.parser.unescape(path)
end
end
@@ -223,14 +223,15 @@ def page_cached?(action)
class ActionCachingTestController < CachingController
rescue_from(Exception) { head 500 }
+ rescue_from(ActionController::UnknownFormat) { head :not_acceptable }
if defined? ActiveRecord
rescue_from(ActiveRecord::RecordNotFound) { head :not_found }
end
# Eliminate uninitialized ivar warning
before_filter { @title = nil }
- caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| !c.request.format.json? }, :expires_in => 1.hour
+ caches_action :index, :redirected, :forbidden, :if => Proc.new { |c| c.request.format && !c.request.format.json? }, :expires_in => 1.hour
caches_action :show, :cache_path => 'http://test.host/custom/show'
caches_action :edit, :cache_path => Proc.new { |c| c.params[:id] ? "http://test.host/#{c.params[:id]};edit" : "http://test.host/edit" }
caches_action :with_layout
@@ -239,6 +240,7 @@ class ActionCachingTestController < CachingController
caches_action :with_layout_proc_param, :layout => Proc.new { |c| c.params[:layout] }
caches_action :record_not_found, :four_oh_four, :simple_runtime_error
caches_action :streaming
+ caches_action :invalid
layout 'talk_from_action'
@@ -303,6 +305,14 @@ def expire_with_url_string
def streaming
render :text => "streaming", :stream => true
end
+
+ def invalid
+ @cache_this = MockTime.now.to_f.to_s
+
+ respond_to do |format|
+ format.json{ render :json => @cache_this }
+ end
+ end
end
class MockTime < Time
@@ -690,6 +700,25 @@ def test_action_caching_plus_streaming
assert fragment_exist?('hostname.com/action_caching_test/streaming')
end
+ def test_invalid_format_returns_not_acceptable
+ get :invalid, :format => "json"
+ assert_response :success
+ cached_time = content_to_cache
+ assert_equal cached_time, @response.body
+
+ assert fragment_exist?("hostname.com/action_caching_test/invalid.json")
+
+ get :invalid, :format => "json"
+ assert_response :success
+ assert_equal cached_time, @response.body
+
+ get :invalid, :format => "xml"
+ assert_response :not_acceptable
+
+ get :invalid, :format => "\xC3\x83"
+ assert_response :not_acceptable
+ end
+
private
def content_to_cache
assigns(:cache_this)

0 comments on commit 79b38c3

Please sign in to comment.