diff options
Diffstat (limited to 'actionpack')
13 files changed, 109 insertions, 78 deletions
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 0727bb8369..04e5922ce8 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -248,6 +248,7 @@ module ActionController MODULES.each do |mod| include mod end + setup_renderer! # Define some internal variables that should not be propagated to the view. PROTECTED_IVARS = AbstractController::Rendering::DEFAULT_PROTECTED_INSTANCE_VARIABLES + [ diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 030a1f3478..0384740fef 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -140,13 +140,6 @@ module ActionController end end - def self.build_with_env(env = {}) #:nodoc: - new.tap { |c| - c.set_request! ActionDispatch::Request.new(env) - c.set_response! make_response!(c.request) - } - end - # Delegates to the class' <tt>controller_name</tt> def controller_name self.class.controller_name diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 4214399b6f..00b551af94 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -11,10 +11,17 @@ module ActionController # Documentation at ActionController::Renderer#render delegate :render, to: :renderer - # Returns a renderer class (inherited from ActionController::Renderer) + # Returns a renderer instance (inherited from ActionController::Renderer) # for the controller. - def renderer - @renderer ||= Renderer.for(self) + attr_reader :renderer + + def setup_renderer! # :nodoc: + @renderer = Renderer.for(self) + end + + def inherited(klass) + klass.setup_renderer! + super end end diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index e5f3cb8e8d..5674eef67b 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -137,8 +137,8 @@ module ActionController #:nodoc: def handle_unverified_request request = @controller.request request.session = NullSessionHash.new(request) - request.env['action_dispatch.request.flash_hash'] = nil - request.env['rack.session.options'] = { skip: true } + request.flash = nil + request.session_options = { skip: true } request.cookie_jar = NullCookieJar.build(request, {}) end diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index bf5c7003ff..903dba3eb4 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -97,9 +97,8 @@ module ActionController # environment they should only be set once at boot-time and never mutated at # runtime. # - # <tt>ActionController::Parameters</tt> inherits from - # <tt>ActiveSupport::HashWithIndifferentAccess</tt>, this means - # that you can fetch values using either <tt>:key</tt> or <tt>"key"</tt>. + # You can fetch values of <tt>ActionController::Parameters</tt> using either + # <tt>:key</tt> or <tt>"key"</tt>. # # params = ActionController::Parameters.new(key: 'value') # params[:key] # => "value" diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index e8b29c5b5e..e4d19e9dba 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -34,67 +34,78 @@ module ActionController # ApplicationController.renderer.new(method: 'post', https: true) # class Renderer - class_attribute :controller, :defaults - # Rack environment to render templates in. - attr_reader :env + attr_reader :defaults, :controller - class << self - delegate :render, to: :new + DEFAULTS = { + http_host: 'example.org', + https: false, + method: 'get', + script_name: '', + input: '' + }.freeze - # Create a new renderer class for a specific controller class. - def for(controller) - Class.new self do - self.controller = controller - self.defaults = { - http_host: 'example.org', - https: false, - method: 'get', - script_name: '', - 'rack.input' => '' - } - end - end + # Create a new renderer instance for a specific controller class. + def self.for(controller, env = {}, defaults = DEFAULTS) + new(controller, env, defaults) + end + + # Create a new renderer for the same controller but with a new env. + def new(env = {}) + self.class.new controller, env, defaults + end + + # Create a new renderer for the same controller but with new defaults. + def with_defaults(defaults) + self.class.new controller, env, self.defaults.merge(defaults) end # Accepts a custom Rack environment to render templates in. # It will be merged with ActionController::Renderer.defaults - def initialize(env = {}) - @env = normalize_keys(defaults).merge normalize_keys(env) - @env['action_dispatch.routes'] = controller._routes + def initialize(controller, env, defaults) + @controller = controller + @defaults = defaults + @env = normalize_keys defaults.merge(env) end # Render templates with any options from ActionController::Base#render_to_string. def render(*args) - raise 'missing controller' unless controller? + raise 'missing controller' unless controller - instance = controller.build_with_env(env) + request = ActionDispatch::Request.new @env + request.routes = controller._routes + + instance = controller.new + instance.set_request! request + instance.set_response! controller.make_response!(request) instance.render_to_string(*args) end private def normalize_keys(env) - http_header_format(env).tap do |new_env| - handle_method_key! new_env - handle_https_key! new_env - end + new_env = {} + env.each_pair { |k,v| new_env[rack_key_for(k)] = rack_value_for(k, v) } + new_env end - def http_header_format(env) - env.transform_keys do |key| - key.is_a?(Symbol) ? key.to_s.upcase : key - end - end + RACK_KEY_TRANSLATION = { + http_host: 'HTTP_HOST', + https: 'HTTPS', + method: 'REQUEST_METHOD', + script_name: 'SCRIPT_NAME', + input: 'rack.input' + } - def handle_method_key!(env) - if method = env.delete('METHOD') - env['REQUEST_METHOD'] = method.upcase - end - end + IDENTITY = ->(_) { _ } + + RACK_VALUE_TRANSLATION = { + https: ->(v) { v ? 'on' : 'off' }, + method: ->(v) { v.upcase }, + } + + def rack_key_for(key); RACK_KEY_TRANSLATION[key]; end - def handle_https_key!(env) - if env.has_key? 'HTTPS' - env['HTTPS'] = env['HTTPS'] ? 'on' : 'off' - end + def rack_value_for(key, value) + RACK_VALUE_TRANSLATION.fetch(key, IDENTITY).call value end end end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 472bb74add..fbbaa1a887 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -78,7 +78,9 @@ module ActionController # params parser middleware, and we should remove this roundtripping # when we switch to caling `call` on the controller - case content_mime_type.ref + case content_mime_type.to_sym + when nil + raise "Unknown Content-Type: #{content_type}" when :json data = ActiveSupport::JSON.encode(non_path_parameters) params = ActiveSupport::JSON.decode(data).with_indifferent_access @@ -90,7 +92,8 @@ module ActionController when :url_encoded_form data = non_path_parameters.to_query else - raise "Unknown Content-Type: #{content_type}" + data = non_path_parameters.to_query + self.request_parameters = non_path_parameters end end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 18504eba6d..b2566c4820 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -324,7 +324,7 @@ module ActionDispatch else self.session = {} end - set_header('action_dispatch.request.flash_hash', nil) + self.flash = nil end def session=(session) #:nodoc: diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index d1e1f1fcf6..45ffacd6f5 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -402,7 +402,7 @@ module ActionDispatch # :nodoc: end def rack_response(status, header) - if NO_CONTENT_CODES.include?(@status) + if NO_CONTENT_CODES.include?(status) header.delete CONTENT_TYPE header.delete 'Content-Length' [status, header, []] diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index 3f7011d100..02b6cfe727 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -53,7 +53,7 @@ module ActionDispatch # # Note that changing the secret key will invalidate all existing sessions! # - # Because CookieStore extends Rack::Session::Abstract::ID, many of the + # Because CookieStore extends Rack::Session::Abstract::Persisted, many of the # options described there can be used to customize the session cookie that # is generated. For example: # diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index b72953f1d1..47f475559a 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -15,7 +15,8 @@ module ActionDispatch # # Configure HSTS with `hsts: { … }`: # * `expires`: How long, in seconds, these settings will stick. Defaults to - # `18.weeks`, the minimum required to qualify for browser preload lists. + # `180.days` (recommended). The minimum required to qualify for browser + # preload lists is `18.weeks`. # * `subdomains`: Set to `true` to tell the browser to apply these settings # to all subdomains. This protects your cookies from interception by a # vulnerable site on a subdomain. Defaults to `false`. diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb index b55a25430b..16d24fa82a 100644 --- a/actionpack/test/controller/renderer_test.rb +++ b/actionpack/test/controller/renderer_test.rb @@ -1,6 +1,10 @@ require 'abstract_unit' class RendererTest < ActiveSupport::TestCase + test 'action controller base has a renderer' do + assert ActionController::Base.renderer + end + test 'creating with a controller' do controller = CommentsController renderer = ActionController::Renderer.for controller @@ -57,8 +61,7 @@ class RendererTest < ActiveSupport::TestCase end test 'rendering with defaults' do - renderer = ApplicationController.renderer - renderer.defaults[:https] = true + renderer = ApplicationController.renderer.new https: true content = renderer.render inline: '<%= request.ssl? %>' assert_equal 'true', content @@ -67,8 +70,8 @@ class RendererTest < ActiveSupport::TestCase test 'same defaults from the same controller' do renderer_defaults = ->(controller) { controller.renderer.defaults } - assert renderer_defaults[AccountsController].equal? renderer_defaults[AccountsController] - assert_not renderer_defaults[AccountsController].equal? renderer_defaults[CommentsController] + assert_equal renderer_defaults[AccountsController], renderer_defaults[AccountsController] + assert_equal renderer_defaults[AccountsController], renderer_defaults[CommentsController] end test 'rendering with different formats' do @@ -83,18 +86,6 @@ class RendererTest < ActiveSupport::TestCase test 'rendering with helpers' do assert_equal "<p>1\n<br />2</p>", render[inline: '<%= simple_format "1\n2" %>'] end - - test 'rendering from inherited renderer' do - inherited = Class.new ApplicationController.renderer do - defaults[:script_name] = 'script' - def render(options) - super options.merge(locals: { param: :value }) - end - end - - template = '<%= url_for controller: :foo, action: :bar, param: param %>' - assert_equal 'script/foo/bar?param=value', inherited.render(inline: template) - end private def render diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index b3c3979c84..06bf9dec74 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -627,6 +627,31 @@ XML assert_equal "application/json", parsed_env["CONTENT_TYPE"] end + def test_mutating_content_type_headers_for_plain_text_files_sets_the_header + @request.headers['Content-Type'] = 'text/plain' + post :render_body, params: { name: 'foo.txt' } + + assert_equal 'text/plain', @request.headers['Content-type'] + assert_equal 'foo.txt', @request.request_parameters[:name] + assert_equal 'render_body', @request.path_parameters[:action] + end + + def test_mutating_content_type_headers_for_html_files_sets_the_header + @request.headers['Content-Type'] = 'text/html' + post :render_body, params: { name: 'foo.html' } + + assert_equal 'text/html', @request.headers['Content-type'] + assert_equal 'foo.html', @request.request_parameters[:name] + assert_equal 'render_body', @request.path_parameters[:action] + end + + def test_mutating_content_type_headers_for_non_registered_mime_type_raises_an_error + assert_raises(RuntimeError) do + @request.headers['Content-Type'] = 'type/fake' + post :render_body, params: { name: 'foo.fake' } + end + end + def test_id_converted_to_string get :test_params, params: { id: 20, foo: Object.new |