diff options
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/CHANGELOG.md | 26 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/exceptions.rb | 8 | ||||
-rw-r--r-- | actionpack/lib/action_controller/metal/rendering.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/mime_type.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/request.rb | 6 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/middleware/reloader.rb | 2 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/middleware/static.rb | 22 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 1 | ||||
-rw-r--r-- | actionpack/test/controller/new_base/render_text_test.rb | 1 | ||||
-rw-r--r-- | actionpack/test/dispatch/request_test.rb | 14 | ||||
-rw-r--r-- | actionpack/test/dispatch/static_test.rb | 35 |
12 files changed, 87 insertions, 31 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 42b4f1d18b..58af62ee76 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,29 @@ +* Show helpful message in `BadRequest` exceptions due to invalid path + parameter encodings. + + Fixes #21923. + + *Agis Anastasopoulos* + +* 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_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 94ec62ec6f..8e040bb465 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -1,6 +1,5 @@ require 'active_support/core_ext/array/extract_options' require 'action_dispatch/middleware/stack' -require 'active_support/deprecation' require 'action_dispatch/http/request' require 'action_dispatch/http/response' diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index 18e003741d..5260dc0336 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -5,12 +5,10 @@ module ActionController class BadRequest < ActionControllerError #:nodoc: attr_reader :original_exception - def initialize(type = nil, e = nil) - return super() unless type && e - - super("Invalid #{type} parameters: #{e.message}") + def initialize(msg = nil, e = nil) + super(msg) @original_exception = e - set_backtrace e.backtrace + set_backtrace e.backtrace if e end end diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 172fbdf954..cce6fe7787 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -1,4 +1,3 @@ -require 'active_support/deprecation' require 'active_support/core_ext/string/filters' module ActionController diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index 95094c25c0..b64f660ec5 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -1,7 +1,6 @@ require 'singleton' require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/string/starts_ends_with' -require 'active_support/deprecation' module Mime class Mimes diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index bf20a33d36..c6ab4dbc9a 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -65,7 +65,7 @@ module ActionDispatch path_parameters.each do |key, value| next unless value.respond_to?(:valid_encoding?) unless value.valid_encoding? - raise ActionController::BadRequest, "Invalid parameter: #{key} => #{value}" + raise ActionController::BadRequest, "Invalid parameter encoding: #{key} => #{value.inspect}" end end end @@ -341,7 +341,7 @@ module ActionDispatch set_header k, Request::Utils.normalize_encode_params(super || {}) end rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e - raise ActionController::BadRequest.new(:query, e) + raise ActionController::BadRequest.new("Invalid query parameters: #{e.message}", e) end alias :query_parameters :GET @@ -357,7 +357,7 @@ module ActionDispatch self.request_parameters = Request::Utils.normalize_encode_params(super || {}) raise rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e - raise ActionController::BadRequest.new(:request, e) + raise ActionController::BadRequest.new("Invalid request parameters: #{e.message}", e) end alias :request_parameters :POST diff --git a/actionpack/lib/action_dispatch/middleware/reloader.rb b/actionpack/lib/action_dispatch/middleware/reloader.rb index 6c7fba00cb..af9a29eb07 100644 --- a/actionpack/lib/action_dispatch/middleware/reloader.rb +++ b/actionpack/lib/action_dispatch/middleware/reloader.rb @@ -1,5 +1,3 @@ -require 'active_support/deprecation/reporting' - module ActionDispatch # ActionDispatch::Reloader provides prepare and cleanup callbacks, # intended to assist with code reloading during development. 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/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 921cda91ee..7c0404ca62 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -3,7 +3,6 @@ require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/enumerable' require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/regexp' -require 'active_support/deprecation' require 'action_dispatch/routing/redirection' require 'action_dispatch/routing/endpoint' diff --git a/actionpack/test/controller/new_base/render_text_test.rb b/actionpack/test/controller/new_base/render_text_test.rb index 435bb18dce..048458178c 100644 --- a/actionpack/test/controller/new_base/render_text_test.rb +++ b/actionpack/test/controller/new_base/render_text_test.rb @@ -1,5 +1,4 @@ require 'abstract_unit' -require 'active_support/deprecation' module RenderText class MinimalController < ActionController::Metal diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index af2ed24f43..dfedc8ae25 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -961,6 +961,20 @@ class RequestParameters < BaseRequestTest end end + test "path parameters with invalid UTF8 encoding" do + request = stub_request( + "action_dispatch.request.path_parameters" => { foo: "\xBE" } + ) + + err = assert_raises(ActionController::BadRequest) do + request.check_path_parameters! + end + + assert_match "Invalid parameter encoding", err.message + assert_match "foo", err.message + assert_match "\\xBE", err.message + end + test "parameters not accessible after rack parse error of invalid UTF8 character" do request = stub_request("QUERY_STRING" => "foo%81E=1") 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 |