From 79b38c386ae3817df5321522ab9ea169c028de96 Mon Sep 17 00:00:00 2001
From: Andrew White <andyw@pixeltrix.co.uk>
Date: Sun, 20 May 2012 19:02:58 +0100
Subject: [PATCH] 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
---
 .../lib/action_controller/caching/actions.rb  |  5 +--
 actionpack/test/controller/caching_test.rb    | 31 ++++++++++++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb
index 80901b8bf3..0238135bc1 100644
--- a/actionpack/lib/action_controller/caching/actions.rb
+++ b/actionpack/lib/action_controller/caching/actions.rb
@@ -47,7 +47,7 @@ module ActionController #:nodoc:
     # 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 @@ module ActionController #:nodoc:
 
       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
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 9efe328d62..d5afef9086 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -223,6 +223,7 @@ end
 
 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
@@ -230,7 +231,7 @@ class ActionCachingTestController < CachingController
   # 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 @@ class ActionCachingTestController < CachingController
   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 @@ class ActionCacheTest < ActionController::TestCase
     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)