Merge pull request #19135 from yuki24/access-control-support

Add basic support for access control headers to ActionDispatch::Static

3 files changed, 60 insertions, 16 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 42b4f1d18b..b9b771b930 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,22 @@
+*   Deprecate `config.static_cache_control` in favor of
+    `config.public_file_server.headers`
+
+    *Yuki Nishijima*
+
+*   Add the ability of returning arbitrary headers to ActionDispatch::Static
+
+    Now ActionDispatch::Static can accept HTTP headers so that developers
+    will have control of returning arbitrary headers like
+    'Access-Control-Allow-Origin' when a response is delivered. They can be
+    configured with `#config`:
+
+      config.public_file_server.headers = {
+        "Cache-Control"               => "public, max-age=60",
+        "Access-Control-Allow-Origin" => "http://rubyonrails.org"
+      }
+
+    *Yuki Nishijima*
+
 *   Allow multiple `root` routes in same scope level. Example:
 
     ```ruby
diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb
index c4344c9609..75f8e05a3f 100644
--- a/actionpack/lib/action_dispatch/middleware/static.rb
+++ b/actionpack/lib/action_dispatch/middleware/static.rb
@@ -3,8 +3,8 @@ require 'active_support/core_ext/uri'
 
 module ActionDispatch
   # This middleware returns a file's contents from disk in the body response.
-  # When initialized, it can accept an optional 'Cache-Control' header, which
-  # will be set when a response containing a file's contents is delivered.
+  # When initialized, it can accept optional HTTP headers, which will be set
+  # when a response containing a file's contents is delivered.
   #
   # This middleware will render the file specified in `env["PATH_INFO"]`
   # where the base path is in the +root+ directory. For example, if the +root+
@@ -13,12 +13,11 @@ module ActionDispatch
   # located at `public/assets/application.js` if the file exists. If the file
   # does not exist, a 404 "File not Found" response will be returned.
   class FileHandler
-    def initialize(root, cache_control, index: 'index')
+    def initialize(root, index: 'index', headers: {})
       @root          = root.chomp('/')
       @compiled_root = /^#{Regexp.escape(root)}/
-      headers        = cache_control && { 'Cache-Control' => cache_control }
-      @file_server = ::Rack::File.new(@root, headers)
-      @index = index
+      @file_server   = ::Rack::File.new(@root, headers)
+      @index         = index
     end
 
     # Takes a path to a file. If the file is found, has valid encoding, and has
@@ -108,9 +107,16 @@ module ActionDispatch
   # produce a directory traversal using this middleware. Only 'GET' and 'HEAD'
   # requests will result in a file being returned.
   class Static
-    def initialize(app, path, cache_control = nil, index: 'index')
+    def initialize(app, path, deprecated_cache_control = :not_set, index: 'index', headers: {})
+      if deprecated_cache_control != :not_set
+        ActiveSupport::Deprecation.warn("The `cache_control` argument is deprecated," \
+                                        "replaced by `headers: { 'Cache-Control' => #{deprecated_cache_control} }`, " \
+                                        " and will be removed in Rails 5.1.")
+        headers['Cache-Control'.freeze] = deprecated_cache_control
+      end
+
       @app = app
-      @file_handler = FileHandler.new(path, cache_control, index: index)
+      @file_handler = FileHandler.new(path, index: index, headers: headers)
     end
 
     def call(env)
diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb
index 13dec8b618..e62ed09b0a 100644
--- a/actionpack/test/dispatch/static_test.rb
+++ b/actionpack/test/dispatch/static_test.rb
@@ -2,6 +2,10 @@ require 'abstract_unit'
 require 'zlib'
 
 module StaticTests
+  DummyApp = lambda { |env|
+    [200, {"Content-Type" => "text/plain"}, ["Hello, World!"]]
+  }
+
   def setup
     silence_warnings do
       @default_internal_encoding = Encoding.default_internal
@@ -37,7 +41,11 @@ module StaticTests
   end
 
   def test_sets_cache_control
-    response = get("/index.html")
+    app = assert_deprecated do
+      ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
+    end
+    response = Rack::MockRequest.new(app).request("GET", "/index.html")
+
     assert_html "/index.html", response
     assert_equal "public, max-age=60", response.headers["Cache-Control"]
   end
@@ -180,6 +188,21 @@ module StaticTests
     assert_equal nil, response.headers['Vary']
   end
 
+  def test_serves_files_with_headers
+    headers = {
+      "Access-Control-Allow-Origin" => 'http://rubyonrails.org',
+      "Cache-Control"               => 'public, max-age=60',
+      "X-Custom-Header"             => "I'm a teapot"
+    }
+
+    app      = ActionDispatch::Static.new(DummyApp, @root, headers: headers)
+    response = Rack::MockRequest.new(app).request("GET", "/foo/bar.html")
+
+    assert_equal 'http://rubyonrails.org', response.headers["Access-Control-Allow-Origin"]
+    assert_equal 'public, max-age=60',     response.headers["Cache-Control"]
+    assert_equal "I'm a teapot",           response.headers["X-Custom-Header"]
+  end
+
   # Windows doesn't allow \ / : * ? " < > | in filenames
   unless RbConfig::CONFIG['host_os'] =~ /mswin|mingw/
     def test_serves_static_file_with_colon
@@ -230,14 +253,10 @@ module StaticTests
 end
 
 class StaticTest < ActiveSupport::TestCase
-  DummyApp = lambda { |env|
-    [200, {"Content-Type" => "text/plain"}, ["Hello, World!"]]
-  }
-
   def setup
     super
     @root = "#{FIXTURE_LOAD_PATH}/public"
-    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
+    @app = ActionDispatch::Static.new(DummyApp, @root, headers: {'Cache-Control' => "public, max-age=60"})
   end
 
   def public_path
@@ -263,7 +282,7 @@ class StaticTest < ActiveSupport::TestCase
   end
 
   def test_non_default_static_index
-    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60", index: "other-index")
+    @app = ActionDispatch::Static.new(DummyApp, @root, index: "other-index")
     assert_html "/other-index.html", get("/other-index.html")
     assert_html "/other-index.html", get("/other-index")
     assert_html "/other-index.html", get("/")
@@ -280,7 +299,7 @@ class StaticEncodingTest < StaticTest
   def setup
     super
     @root = "#{FIXTURE_LOAD_PATH}/公共"
-    @app = ActionDispatch::Static.new(DummyApp, @root, "public, max-age=60")
+    @app = ActionDispatch::Static.new(DummyApp, @root, headers: {'Cache-Control' => "public, max-age=60"})
   end
 
   def public_path