Set X-Accel-Redirect to percent-encoded path

2 files changed, 18 insertions, 4 deletions

diff --git a/lib/rack/sendfile.rb b/lib/rack/sendfile.rb
index 6113f858..51ba4db5 100644
--- a/lib/rack/sendfile.rb
+++ b/lib/rack/sendfile.rb
@@ -117,7 +117,8 @@ module Rack
           path = ::File.expand_path(body.to_path)
           if url = map_accel_path(env, path)
             headers[CONTENT_LENGTH] = '0'
-            headers[type] = url
+            # '?' must be percent-encoded because it is not query string but a part of path
+            headers[type] = ::Rack::Utils.escape_path(url).gsub('?', '%3F')
             obody = body
             body = Rack::BodyProxy.new([]) do
               obody.close if obody.respond_to?(:close)
diff --git a/test/spec_sendfile.rb b/test/spec_sendfile.rb
index cae458e4..8688df9f 100644
--- a/test/spec_sendfile.rb
+++ b/test/spec_sendfile.rb
@@ -8,10 +8,10 @@ require 'rack/mock'
 require 'tmpdir'
 
 describe Rack::Sendfile do
-  def sendfile_body
-    FileUtils.touch File.join(Dir.tmpdir,  "rack_sendfile")
+  def sendfile_body(filename = "rack_sendfile")
+    FileUtils.touch File.join(Dir.tmpdir,  filename)
     res = ['Hello World']
-    def res.to_path ; File.join(Dir.tmpdir,  "rack_sendfile") ; end
+    res.define_singleton_method(:to_path) { File.join(Dir.tmpdir,  filename) }
     res
   end
 
@@ -74,6 +74,19 @@ describe Rack::Sendfile do
     end
   end
 
+  it "sets X-Accel-Redirect response header to percent-encoded path" do
+    headers = {
+      'HTTP_X_SENDFILE_TYPE' => 'X-Accel-Redirect',
+      'HTTP_X_ACCEL_MAPPING' => "#{Dir.tmpdir}/=/foo/bar%/"
+    }
+    request headers, sendfile_body('file_with_%_?_symbol') do |response|
+      response.must_be :ok?
+      response.body.must_be :empty?
+      response.headers['Content-Length'].must_equal '0'
+      response.headers['X-Accel-Redirect'].must_equal '/foo/bar%25/file_with_%25_%3F_symbol'
+    end
+  end
+
   it 'writes to rack.error when no X-Accel-Mapping is specified' do
     request 'HTTP_X_SENDFILE_TYPE' => 'X-Accel-Redirect' do |response|
       response.must_be :ok?