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

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

8 files changed, 106 insertions, 22 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
diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb
index 75112f29b6..ee9c87b5cf 100644
--- a/railties/lib/rails/application/configuration.rb
+++ b/railties/lib/rails/application/configuration.rb
@@ -11,12 +11,12 @@ module Rails
                     :eager_load, :exceptions_app, :file_watcher, :filter_parameters,
                     :force_ssl, :helpers_paths, :logger, :log_formatter, :log_tags,
                     :railties_order, :relative_url_root, :secret_key_base, :secret_token,
-                    :serve_static_files, :ssl_options, :static_cache_control, :static_index,
+                    :serve_static_files, :ssl_options, :static_index, :public_file_server,
                     :session_options, :time_zone, :reload_classes_only_on_change,
                     :beginning_of_week, :filter_redirect, :x
 
       attr_writer :log_level
-      attr_reader :encoding, :api_only
+      attr_reader :encoding, :api_only, :static_cache_control
 
       def initialize(*)
         super
@@ -27,8 +27,8 @@ module Rails
         @filter_redirect               = []
         @helpers_paths                 = []
         @serve_static_files            = true
-        @static_cache_control          = nil
         @static_index                  = "index"
+        @public_file_server            = ActiveSupport::OrderedOptions.new
         @force_ssl                     = false
         @ssl_options                   = {}
         @session_store                 = :cookie_store
@@ -52,6 +52,14 @@ module Rails
         @x                             = Custom.new
       end
 
+      def static_cache_control=(value)
+        ActiveSupport::Deprecation.warn("static_cache_control is deprecated and will be removed in Rails 5.1. " \
+                                        "Please use `config.public_file_server.headers = {'Cache-Control' => #{value}} " \
+                                        "instead.")
+
+        @static_cache_control = value
+      end
+
       def encoding=(value)
         @encoding = value
         silence_warnings do
diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb
index 21062f3a53..387d92db73 100644
--- a/railties/lib/rails/application/default_middleware_stack.rb
+++ b/railties/lib/rails/application/default_middleware_stack.rb
@@ -18,7 +18,10 @@ module Rails
           middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header
 
           if config.serve_static_files
-            middleware.use ::ActionDispatch::Static, paths["public"].first, config.static_cache_control, index: config.static_index
+            headers = config.public_file_server.headers || {}
+            headers['Cache-Control'.freeze] = config.static_cache_control if config.static_cache_control
+
+            middleware.use ::ActionDispatch::Static, paths["public"].first, index: config.static_index, headers: headers
           end
 
           if rack_cache = load_rack_cache
diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt
index 0306deb18c..5165100c22 100644
--- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt
+++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt
@@ -13,8 +13,10 @@ Rails.application.configure do
   config.eager_load = false
 
   # Configure static file server for tests with Cache-Control for performance.
-  config.serve_static_files   = true
-  config.static_cache_control = 'public, max-age=3600'
+  config.serve_static_files         = true
+  config.public_file_server.headers = {
+    'Cache-Control' => 'public, max-age=3600'
+  }
 
   # Show full error reports and disable caching.
   config.consider_all_requests_local       = true
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
index 2f407cd851..d96d8ded6b 100644
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@@ -339,6 +339,16 @@ module ApplicationTests
       end
     end
 
+    test "config.static_cache_control is deprecated" do
+      make_basic_app do |application|
+        assert_deprecated do
+          application.config.static_cache_control = "public, max-age=60"
+        end
+
+        assert_equal application.config.static_cache_control, "public, max-age=60"
+      end
+    end
+
     test "Use key_generator when secret_key_base is set" do
       make_basic_app do |application|
         application.secrets.secret_key_base = 'b3c631c314c0bbca50c1b2843150fe33'
diff --git a/railties/test/application/middleware/static_test.rb b/railties/test/application/middleware/static_test.rb
index 1a46cd3568..5366537dc2 100644
--- a/railties/test/application/middleware/static_test.rb
+++ b/railties/test/application/middleware/static_test.rb
@@ -27,6 +27,23 @@ module ApplicationTests
       assert_not last_response.headers.has_key?('Cache-Control'), "Cache-Control should not be set"
     end
 
+    test "headers for static files are configurable" do
+      app_file "public/about.html", 'static'
+      add_to_config <<-CONFIG
+        config.public_file_server.headers = {
+          "Access-Control-Allow-Origin" => "http://rubyonrails.org",
+          "Cache-Control"               => "public, max-age=60"
+        }
+      CONFIG
+
+      require "#{app_path}/config/environment"
+
+      get '/about.html'
+
+      assert_equal 'http://rubyonrails.org', last_response.headers["Access-Control-Allow-Origin"]
+      assert_equal 'public, max-age=60',     last_response.headers["Cache-Control"]
+    end
+
     test "static_index defaults to 'index'" do
       app_file "public/index.html", "/index.html"