diff options
Diffstat (limited to 'actionpack')
34 files changed, 648 insertions, 503 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1cfd633606..73a833a23b 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,34 @@ +* `url_for` does not modify its arguments when generating polymorphic URLs. + + *Bernerd Schaefer* + +* Make it easier to opt in to `config.force_ssl` and `config.ssl_options` by + making them less dangerous to try and easier to disable. + + SSL redirect: + * Move `:host` and `:port` options within `redirect: { … }`. Deprecate. + * Introduce `:status` and `:body` to customize the redirect response. + The 301 permanent default makes it difficult to test the redirect and + back out of it since browsers remember the 301. Test with a 302 or 307 + instead, then switch to 301 once you're confident that all is well. + + HTTP Strict Transport Security (HSTS): + * Shorter max-age. Shorten the default max-age from 1 year to 180 days, + the low end for https://www.ssllabs.com/ssltest/ grading and greater + than the 18-week minimum to qualify for browser preload lists. + * Disabling HSTS. Setting `hsts: false` now sets `hsts { expires: 0 }` + instead of omitting the header. Omitting does nothing to disable HSTS + since browsers hang on to your previous settings until they expire. + Sending `{ hsts: { expires: 0 }}` flushes out old browser settings and + actually disables HSTS: + http://tools.ietf.org/html/rfc6797#section-6.1.1 + * HSTS Preload. Introduce `preload: true` to set the `preload` flag, + indicating that your site may be included in browser preload lists, + including Chrome, Firefox, Safari, IE11, and Edge. Submit your site: + https://hstspreload.appspot.com + + *Jeremy Daer* + * Update `ActionController::TestSession#fetch` to behave more like `ActionDispatch::Request::Session#fetch` when using non-string keys. diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 6db0941b52..6c0a072b73 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -24,9 +24,9 @@ module AbstractController options = _normalize_render(*args, &block) self.response_body = render_to_body(options) if options[:html] - _set_content_type Mime::HTML.to_s + _set_html_content_type else - _set_content_type _get_content_type(rendered_format) + _set_rendered_content_type rendered_format end self.response_body end @@ -106,11 +106,10 @@ module AbstractController def _process_format(format) end - def _get_content_type(rendered_format) # :nodoc: - rendered_format.to_s + def _set_html_content_type # :nodoc: end - def _set_content_type(type) # :nodoc: + def _set_rendered_content_type(format) # :nodoc: end # Normalize args and options. 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/live.rb b/actionpack/lib/action_controller/metal/live.rb index 69583f8ab4..667c7f87ca 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -145,8 +145,8 @@ module ActionController def write(string) unless @response.committed? - @response.headers["Cache-Control"] = "no-cache" - @response.headers.delete "Content-Length" + @response.set_header "Cache-Control", "no-cache" + @response.delete_header "Content-Length" end super diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index e62da0fa70..88a4818c16 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -191,7 +191,7 @@ module ActionController #:nodoc: if format = collector.negotiate_format(request) _process_format(format) - _set_content_type _get_content_type format + _set_rendered_content_type format response = collector.response response ? response.call : render({}) else diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index c8934b367f..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 @@ -56,12 +63,14 @@ module ActionController nil end - def _get_content_type(rendered_format) - self.content_type || super + def _set_html_content_type + self.content_type = Mime::HTML.to_s end - def _set_content_type(format) - self.content_type = format + def _set_rendered_content_type(format) + unless response.content_type + self.content_type = format.to_s + end end # Normalize arguments by catching blocks and setting them on :update. 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 4aee489912..45ffacd6f5 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -38,8 +38,6 @@ module ActionDispatch # :nodoc: # The HTTP status code. attr_reader :status - attr_writer :sending_file - # Get headers for this response. attr_reader :header @@ -48,20 +46,6 @@ module ActionDispatch # :nodoc: delegate :[], :[]=, :to => :@header delegate :each, :to => :@stream - # Sets the HTTP response's content MIME type. For example, in the controller - # you could write this: - # - # response.content_type = "text/plain" - # - # If a character set has been defined for this response (see charset=) then - # the character set information will also be included in the content type - # information. - attr_reader :content_type - - # The charset of the response. HTML wants to know the encoding of the - # content you're giving them, so we need to send that along. - attr_reader :charset - CONTENT_TYPE = "Content-Type".freeze SET_COOKIE = "Set-Cookie".freeze LOCATION = "Location".freeze @@ -130,20 +114,11 @@ module ActionDispatch # :nodoc: self.body, self.status = body, status - @sending_file = false @blank = false @cv = new_cond @committed = false @sending = false @sent = false - @content_type = nil - @charset = self.class.default_charset - - if content_type = self[CONTENT_TYPE] - type, charset = content_type.split(/;\s*charset=/) - @content_type = Mime::Type.lookup(type) - @charset = charset || self.class.default_charset - end prepare_cache_control! @@ -199,7 +174,27 @@ module ActionDispatch # :nodoc: # Sets the HTTP content type. def content_type=(content_type) - @content_type = content_type.to_s + header_info = parse_content_type + set_content_type content_type.to_s, header_info.charset || self.class.default_charset + end + + # Sets the HTTP response's content MIME type. For example, in the controller + # you could write this: + # + # response.content_type = "text/plain" + # + # If a character set has been defined for this response (see charset=) then + # the character set information will also be included in the content type + # information. + + def content_type + parse_content_type.mime_type + end + + def sending_file=(v) + if true == v + self.charset = false + end end # Sets the HTTP character set. In case of nil parameter @@ -208,7 +203,20 @@ module ActionDispatch # :nodoc: # response.charset = 'utf-16' # => 'utf-16' # response.charset = nil # => 'utf-8' def charset=(charset) - @charset = charset.nil? ? self.class.default_charset : charset + header_info = parse_content_type + if false == charset + set_header CONTENT_TYPE, header_info.mime_type + else + content_type = header_info.mime_type + set_content_type content_type, charset || self.class.default_charset + end + end + + # The charset of the response. HTML wants to know the encoding of the + # content you're giving them, so we need to send that along. + def charset + header_info = parse_content_type + header_info.charset || self.class.default_charset end # The response code of the request. @@ -308,6 +316,26 @@ module ActionDispatch # :nodoc: private + ContentTypeHeader = Struct.new :mime_type, :charset + NullContentTypeHeader = ContentTypeHeader.new nil, nil + + def parse_content_type + content_type = get_header CONTENT_TYPE + if content_type + type, charset = content_type.split(/;\s*charset=/) + type = nil if type.empty? + ContentTypeHeader.new(type, charset) + else + NullContentTypeHeader + end + end + + def set_content_type(content_type, charset) + type = (content_type || '').dup + type << "; charset=#{charset}" if charset + set_header CONTENT_TYPE, type + end + def before_committed return if committed? assign_default_content_type_and_charset! @@ -330,18 +358,11 @@ module ActionDispatch # :nodoc: end def assign_default_content_type_and_charset! - return if get_header(CONTENT_TYPE).present? - - @content_type ||= Mime::HTML - - type = @content_type.to_s.dup - type << "; charset=#{charset}" if append_charset? - - set_header CONTENT_TYPE, type - end + return if content_type - def append_charset? - !@sending_file && @charset != false + ct = parse_content_type + set_content_type(ct.mime_type || Mime::HTML.to_s, + ct.charset || self.class.default_charset) end class RackBody @@ -381,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/journey/nfa/dot.rb b/actionpack/lib/action_dispatch/journey/nfa/dot.rb index 47bf76bdbf..7063b44bb5 100644 --- a/actionpack/lib/action_dispatch/journey/nfa/dot.rb +++ b/actionpack/lib/action_dispatch/journey/nfa/dot.rb @@ -1,5 +1,3 @@ -# encoding: utf-8 - module ActionDispatch module Journey # :nodoc: module NFA # :nodoc: diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 537c9b2f5c..306d2e674a 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -1,5 +1,3 @@ -# encoding: utf-8 - module ActionDispatch module Journey # :nodoc: class Format diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index f37439e4d7..b653e4eacd 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -221,19 +221,11 @@ module ActionDispatch end end - protected - - def request; @parent_jar.request; end - private def upgrade_legacy_signed_cookies? request.secret_token.present? && request.secret_key_base.present? end - - def key_generator - request.key_generator - end end # Passing the ActiveSupport::MessageEncryptor::NullSerializer downstream @@ -253,6 +245,11 @@ module ActionDispatch rescue ActiveSupport::MessageVerifier::InvalidSignature nil end + + private + def parse(name, signed_message) + super || verify_and_upgrade_legacy_signed_message(name, signed_message) + end end class CookieJar #:nodoc: @@ -412,7 +409,7 @@ module ActionDispatch end end - class PermanentCookieJar #:nodoc: + class AbstractCookieJar # :nodoc: include ChainedCookieJars def initialize(parent_jar) @@ -420,19 +417,35 @@ module ActionDispatch end def [](name) - @parent_jar[name.to_s] + if data = @parent_jar[name.to_s] + parse name, data + end end def []=(name, options) if options.is_a?(Hash) options.symbolize_keys! else - options = { :value => options } + options = { value: options } end - options[:expires] = 20.years.from_now + commit(options) @parent_jar[name] = options end + + protected + def request; @parent_jar.request; end + + private + def parse(name, data); data; end + def commit(options); end + end + + class PermanentCookieJar < AbstractCookieJar # :nodoc: + private + def commit(options) + options[:expires] = 20.years.from_now + end end class JsonSerializer # :nodoc: @@ -484,45 +497,30 @@ module ActionDispatch def digest request.cookies_digest || 'SHA1' end + + def key_generator + request.key_generator + end end - class SignedCookieJar #:nodoc: - include ChainedCookieJars + class SignedCookieJar < AbstractCookieJar # :nodoc: include SerializedCookieJars def initialize(parent_jar) - @parent_jar = parent_jar + super secret = key_generator.generate_key(request.signed_cookie_salt) @verifier = ActiveSupport::MessageVerifier.new(secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end - # Returns the value of the cookie by +name+ if it is untampered, - # returns +nil+ otherwise or if no such cookie exists. - def [](name) - if signed_message = @parent_jar[name] - deserialize name, verify(signed_message) + private + def parse(name, signed_message) + deserialize name, @verifier.verified(signed_message) end - end - # Signs and sets the cookie named +name+. The second argument may be the cookie's - # value or a hash of options as documented above. - def []=(name, options) - if options.is_a?(Hash) - options.symbolize_keys! + def commit(options) options[:value] = @verifier.generate(serialize(options[:value])) - else - options = { :value => @verifier.generate(serialize(options)) } - end - raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE - @parent_jar[name] = options - end - - private - def verify(signed_message) - @verifier.verify(signed_message) - rescue ActiveSupport::MessageVerifier::InvalidSignature - nil + raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE end end @@ -532,20 +530,13 @@ module ActionDispatch # re-saves them using the new key generator to provide a smooth upgrade path. class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc: include VerifyAndUpgradeLegacySignedMessage - - def [](name) - if signed_message = @parent_jar[name] - deserialize(name, verify(signed_message)) || verify_and_upgrade_legacy_signed_message(name, signed_message) - end - end end - class EncryptedCookieJar #:nodoc: - include ChainedCookieJars + class EncryptedCookieJar < AbstractCookieJar # :nodoc: include SerializedCookieJars def initialize(parent_jar) - @parent_jar = parent_jar + super if ActiveSupport::LegacyKeyGenerator === key_generator raise "You didn't set secrets.secret_key_base, which is required for this cookie jar. " + @@ -557,35 +548,18 @@ module ActionDispatch @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, digest: digest, serializer: ActiveSupport::MessageEncryptor::NullSerializer) end - # Returns the value of the cookie by +name+ if it is untampered, - # returns +nil+ otherwise or if no such cookie exists. - def [](name) - if encrypted_message = @parent_jar[name] - deserialize name, decrypt_and_verify(encrypted_message) - end - end - - # Encrypts and sets the cookie named +name+. The second argument may be the cookie's - # value or a hash of options as documented above. - def []=(name, options) - if options.is_a?(Hash) - options.symbolize_keys! - else - options = { :value => options } - end - - options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value])) - - raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE - @parent_jar[name] = options - end - private - def decrypt_and_verify(encrypted_message) - @encryptor.decrypt_and_verify(encrypted_message) + def parse(name, encrypted_message) + deserialize name, @encryptor.decrypt_and_verify(encrypted_message) rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage nil end + + def commit(options) + options[:value] = @encryptor.encrypt_and_sign(serialize(options[:value])) + + raise CookieOverflow if options[:value].bytesize > MAX_COOKIE_SIZE + end end # UpgradeLegacyEncryptedCookieJar is used by ActionDispatch::Session::CookieStore @@ -594,12 +568,6 @@ module ActionDispatch # encrypts and re-saves them using the new key generator to provide a smooth upgrade path. class UpgradeLegacyEncryptedCookieJar < EncryptedCookieJar #:nodoc: include VerifyAndUpgradeLegacySignedMessage - - def [](name) - if encrypted_or_signed_message = @parent_jar[name] - deserialize(name, decrypt_and_verify(encrypted_or_signed_message)) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message) - end - end end def initialize(app) 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 7b3d8bcc5b..47f475559a 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -1,72 +1,129 @@ module ActionDispatch + # This middleware is added to the stack when `config.force_ssl = true`. + # It does three jobs to enforce secure HTTP requests: + # + # 1. TLS redirect. http:// requests are permanently redirected to https:// + # with the same URL host, path, etc. Pass `:host` and/or `:port` to + # modify the destination URL. This is always enabled. + # + # 2. Secure cookies. Sets the `secure` flag on cookies to tell browsers they + # mustn't be sent along with http:// requests. This is always enabled. + # + # 3. HTTP Strict Transport Security (HSTS). Tells the browser to remember + # this site as TLS-only and automatically redirect non-TLS requests. + # Enabled by default. Pass `hsts: false` to disable. + # + # Configure HSTS with `hsts: { … }`: + # * `expires`: How long, in seconds, these settings will stick. Defaults to + # `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`. + # * `preload`: Advertise that this site may be included in browsers' + # preloaded HSTS lists. HSTS protects your site on every visit *except the + # first visit* since it hasn't seen your HSTS header yet. To close this + # gap, browser vendors include a baked-in list of HSTS-enabled sites. + # Go to https://hstspreload.appspot.com to submit your site for inclusion. + # + # Disabling HSTS: To turn off HSTS, omitting the header is not enough. + # Browsers will remember the original HSTS directive until it expires. + # Instead, use the header to tell browsers to expire HSTS immediately. + # Setting `hsts: false` is a shortcut for `hsts: { expires: 0 }`. class SSL - YEAR = 31536000 + # Default to 180 days, the low end for https://www.ssllabs.com/ssltest/ + # and greater than the 18-week requirement for browser preload lists. + HSTS_EXPIRES_IN = 15552000 def self.default_hsts_options - { :expires => YEAR, :subdomains => false } + { expires: HSTS_EXPIRES_IN, subdomains: false, preload: false } end - def initialize(app, options = {}) + def initialize(app, redirect: {}, hsts: {}, **options) @app = app - @hsts = options.fetch(:hsts, {}) - @hsts = {} if @hsts == true - @hsts = self.class.default_hsts_options.merge(@hsts) if @hsts + if options[:host] || options[:port] + ActiveSupport::Deprecation.warn <<-end_warning.strip_heredoc + The `:host` and `:port` options are moving within `:redirect`: + `config.ssl_options = { redirect: { host: …, port: … }}`. + end_warning + @redirect = options.slice(:host, :port) + else + @redirect = redirect + end - @host = options[:host] - @port = options[:port] + @hsts_header = build_hsts_header(normalize_hsts_options(hsts)) end def call(env) - request = Request.new(env) + request = Request.new env if request.ssl? - status, headers, body = @app.call(env) - headers.reverse_merge!(hsts_headers) - flag_cookies_as_secure!(headers) - [status, headers, body] + @app.call(env).tap do |status, headers, body| + set_hsts_header! headers + flag_cookies_as_secure! headers + end else - redirect_to_https(request) + redirect_to_https request end end private - def redirect_to_https(request) - host = @host || request.host - port = @port || request.port - - location = "https://#{host}" - location << ":#{port}" if port != 80 - location << request.fullpath - - headers = { 'Content-Type' => 'text/html', 'Location' => location } - - [301, headers, []] + def set_hsts_header!(headers) + headers['Strict-Transport-Security'.freeze] ||= @hsts_header end - # http://tools.ietf.org/html/draft-hodges-strict-transport-sec-02 - def hsts_headers - if @hsts - value = "max-age=#{@hsts[:expires].to_i}" - value += "; includeSubDomains" if @hsts[:subdomains] - { 'Strict-Transport-Security' => value } + def normalize_hsts_options(options) + case options + # Explicitly disabling HSTS clears the existing setting from browsers + # by setting expiry to 0. + when false + self.class.default_hsts_options.merge(expires: 0) + # Default to enabled, with default options. + when nil, true + self.class.default_hsts_options else - {} + self.class.default_hsts_options.merge(options) end end + # http://tools.ietf.org/html/rfc6797#section-6.1 + def build_hsts_header(hsts) + value = "max-age=#{hsts[:expires].to_i}" + value << "; includeSubDomains" if hsts[:subdomains] + value << "; preload" if hsts[:preload] + value + end + def flag_cookies_as_secure!(headers) - if cookies = headers['Set-Cookie'] - cookies = cookies.split("\n") + if cookies = headers['Set-Cookie'.freeze] + cookies = cookies.split("\n".freeze) - headers['Set-Cookie'] = cookies.map { |cookie| + headers['Set-Cookie'.freeze] = cookies.map { |cookie| if cookie !~ /;\s*secure\s*(;|$)/i "#{cookie}; secure" else cookie end - }.join("\n") + }.join("\n".freeze) end end + + def redirect_to_https(request) + [ @redirect.fetch(:status, 301), + { 'Content-Type' => 'text/html', + 'Location' => https_location_for(request) }, + @redirect.fetch(:body, []) ] + end + + def https_location_for(request) + host = @redirect[:host] || request.host + port = @redirect[:port] || request.port + + location = "https://#{host}" + location << ":#{port}" if port != 80 && port != 443 + location << request.fullpath + location + end end end diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 8757c9ea7f..7182ae201c 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -1,5 +1,3 @@ -# encoding: UTF-8 - module ActionDispatch # The routing module provides URL rewriting in native Ruby. It's a way to # redirect incoming requests to controllers and actions. This replaces diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 967bbd62f8..883cd9c2c3 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -180,7 +180,8 @@ module ActionDispatch when Symbol HelperMethodBuilder.url.handle_string_call self, options when Array - polymorphic_url(options, options.extract_options!) + components = options.dup + polymorphic_url(components, components.extract_options!) when Class HelperMethodBuilder.url.handle_class_call self, options else diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 1954324222..3c498960e4 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -406,7 +406,6 @@ def jruby_skip(message = '') skip message if defined?(JRUBY_VERSION) end -require 'mocha/setup' # FIXME: stop using mocha require 'active_support/testing/method_call_assertions' class ForkingExecutor diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 5698159eba..bc0ffd3eaa 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -299,30 +299,42 @@ class CacheHelperOutputBufferTest < ActionController::TestCase def test_output_buffer output_buffer = ActionView::OutputBuffer.new controller = MockController.new - cache_helper = Object.new + cache_helper = Class.new do + def self.controller; end; + def self.output_buffer; end; + def self.output_buffer=; end; + end cache_helper.extend(ActionView::Helpers::CacheHelper) - cache_helper.expects(:controller).returns(controller).at_least(0) - cache_helper.expects(:output_buffer).returns(output_buffer).at_least(0) - # if the output_buffer is changed, the new one should be html_safe and of the same type - cache_helper.expects(:output_buffer=).with(responds_with(:html_safe?, true)).with(instance_of(output_buffer.class)).at_least(0) - assert_nothing_raised do - cache_helper.send :fragment_for, 'Test fragment name', 'Test fragment', &Proc.new{ nil } + cache_helper.stub :controller, controller do + cache_helper.stub :output_buffer, output_buffer do + assert_called_with cache_helper, :output_buffer=, [output_buffer.class.new(output_buffer)] do + assert_nothing_raised do + cache_helper.send :fragment_for, 'Test fragment name', 'Test fragment', &Proc.new{ nil } + end + end + end end end def test_safe_buffer output_buffer = ActiveSupport::SafeBuffer.new controller = MockController.new - cache_helper = Object.new + cache_helper = Class.new do + def self.controller; end; + def self.output_buffer; end; + def self.output_buffer=; end; + end cache_helper.extend(ActionView::Helpers::CacheHelper) - cache_helper.expects(:controller).returns(controller).at_least(0) - cache_helper.expects(:output_buffer).returns(output_buffer).at_least(0) - # if the output_buffer is changed, the new one should be html_safe and of the same type - cache_helper.expects(:output_buffer=).with(responds_with(:html_safe?, true)).with(instance_of(output_buffer.class)).at_least(0) - assert_nothing_raised do - cache_helper.send :fragment_for, 'Test fragment name', 'Test fragment', &Proc.new{ nil } + cache_helper.stub :controller, controller do + cache_helper.stub :output_buffer, output_buffer do + assert_called_with cache_helper, :output_buffer=, [output_buffer.class.new(output_buffer)] do + assert_nothing_raised do + cache_helper.send :fragment_for, 'Test fragment name', 'Test fragment', &Proc.new{ nil } + end + end + end end end end diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb index 8bf016d060..7226beed26 100644 --- a/actionpack/test/controller/params_wrapper_test.rb +++ b/actionpack/test/controller/params_wrapper_test.rb @@ -28,8 +28,17 @@ class ParamsWrapperTest < ActionController::TestCase end end - class User; end - class Person; end + class User + def self.attribute_names + [] + end + end + + class Person + def self.attribute_names + [] + end + end tests UsersController @@ -155,33 +164,28 @@ class ParamsWrapperTest < ActionController::TestCase end def test_derived_wrapped_keys_from_matching_model - User.expects(:respond_to?).with(:attribute_names).returns(true) - User.expects(:attribute_names).twice.returns(["username"]) - - with_default_wrapper_options do - @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } - assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) + assert_called(User, :attribute_names, times: 2, returns: ["username"]) do + with_default_wrapper_options do + @request.env['CONTENT_TYPE'] = 'application/json' + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } + assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'user' => { 'username' => 'sikachu' }}) + end end end def test_derived_wrapped_keys_from_specified_model with_default_wrapper_options do - Person.expects(:respond_to?).with(:attribute_names).returns(true) - Person.expects(:attribute_names).twice.returns(["username"]) + assert_called(Person, :attribute_names, times: 2, returns: ["username"]) do + UsersController.wrap_parameters Person - UsersController.wrap_parameters Person - - @request.env['CONTENT_TYPE'] = 'application/json' - post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } - assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }}) + @request.env['CONTENT_TYPE'] = 'application/json' + post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } + assert_parameters({ 'username' => 'sikachu', 'title' => 'Developer', 'person' => { 'username' => 'sikachu' }}) + end end end def test_not_wrapping_abstract_model - User.expects(:respond_to?).with(:attribute_names).returns(true) - User.expects(:attribute_names).returns([]) - with_default_wrapper_options do @request.env['CONTENT_TYPE'] = 'application/json' post :parse, params: { 'username' => 'sikachu', 'title' => 'Developer' } 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/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb index 90fd8669c2..94ffbe3cd0 100644 --- a/actionpack/test/controller/request_forgery_protection_test.rb +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -379,7 +379,6 @@ module RequestForgeryProtectionTests end def test_should_not_raise_error_if_token_is_not_a_string - @controller.unstub(:valid_authenticity_token?) assert_blocked do patch :index, params: { custom_authenticity_token: { foo: 'bar' } } end diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index e767323773..f53f061e10 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -246,12 +246,15 @@ class RescueControllerTest < ActionController::TestCase end def test_rescue_handler_with_argument - @controller.expects(:show_errors).once.with { |e| e.is_a?(Exception) } - get :record_invalid + assert_called_with @controller, :show_errors, [Exception] do + get :record_invalid + end end + def test_rescue_handler_with_argument_as_string - @controller.expects(:show_errors).once.with { |e| e.is_a?(Exception) } - get :record_invalid_raise_as_string + assert_called_with @controller, :show_errors, [Exception] do + get :record_invalid_raise_as_string + end end def test_proc_rescue_handler 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 diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index 31677f202d..78e883f134 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -451,6 +451,26 @@ module AbstractController end end + def test_url_for_with_array_is_unmodified + with_routing do |set| + set.draw do + namespace :admin do + resources :posts + end + end + + kls = Class.new { include set.url_helpers } + kls.default_url_options[:host] = 'www.basecamphq.com' + + original_components = [:new, :admin, :post, { param: 'value' }] + components = original_components.dup + + kls.new.url_for(components) + + assert_equal(original_components, components) + end + end + private def extract_params(url) url.split('?', 2).last.split('&').sort diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index f9f379780c..93258fbceb 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -272,9 +272,12 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest test 'uses backtrace cleaner from env' do @app = DevelopmentApp - cleaner = stub(:clean => ['passed backtrace cleaner']) - get "/", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => cleaner } - assert_match(/passed backtrace cleaner/, body) + backtrace_cleaner = ActiveSupport::BacktraceCleaner.new + + backtrace_cleaner.stub :clean, ['passed backtrace cleaner'] do + get "/", headers: { 'action_dispatch.show_exceptions' => true, 'action_dispatch.backtrace_cleaner' => backtrace_cleaner } + assert_match(/passed backtrace cleaner/, body) + end end test 'logs exception backtrace when all lines silenced' do diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index f37cce4d45..dfbb91c0ca 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -25,27 +25,29 @@ module ActionDispatch exception = TestError.new("lib/file.rb:42:in `index'") wrapper = ExceptionWrapper.new(nil, exception) - wrapper.expects(:source_fragment).with('lib/file.rb', 42).returns('foo') - - assert_equal [ code: 'foo', line_number: 42 ], wrapper.source_extracts + assert_called_with(wrapper, :source_fragment, ['lib/file.rb', 42], returns: 'foo') do + assert_equal [ code: 'foo', line_number: 42 ], wrapper.source_extracts + end end test '#source_extracts works with Windows paths' do exc = TestError.new("c:/path/to/rails/app/controller.rb:27:in 'index':") wrapper = ExceptionWrapper.new(nil, exc) - wrapper.expects(:source_fragment).with('c:/path/to/rails/app/controller.rb', 27).returns('nothing') - assert_equal [ code: 'nothing', line_number: 27 ], wrapper.source_extracts + assert_called_with(wrapper, :source_fragment, ['c:/path/to/rails/app/controller.rb', 27], returns: 'nothing') do + assert_equal [ code: 'nothing', line_number: 27 ], wrapper.source_extracts + end end test '#source_extracts works with non standard backtrace' do exc = TestError.new('invalid') wrapper = ExceptionWrapper.new(nil, exc) - wrapper.expects(:source_fragment).with('invalid', 0).returns('nothing') - assert_equal [ code: 'nothing', line_number: 0 ], wrapper.source_extracts + assert_called_with(wrapper, :source_fragment, ['invalid', 0], returns: 'nothing') do + assert_equal [ code: 'nothing', line_number: 0 ], wrapper.source_extracts + end end test '#application_trace returns traces only from the application' do diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index ff63c10e8d..258d097b7c 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -749,20 +749,23 @@ end class RequestFormat < BaseRequestTest test "xml format" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => 'xml' }) - assert_equal Mime::XML, request.format + assert_called(request, :parameters, times: 2, returns: {format: :xml}) do + assert_equal Mime::XML, request.format + end end test "xhtml format" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => 'xhtml' }) - assert_equal Mime::HTML, request.format + assert_called(request, :parameters, times: 2, returns: {format: :xhtml}) do + assert_equal Mime::HTML, request.format + end end test "txt format" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => 'txt' }) - assert_equal Mime::TEXT, request.format + assert_called(request, :parameters, times: 2, returns: {format: :txt}) do + assert_equal Mime::TEXT, request.format + end end test "XMLHttpRequest" do @@ -770,21 +773,25 @@ class RequestFormat < BaseRequestTest 'HTTP_X_REQUESTED_WITH' => 'XMLHttpRequest', 'HTTP_ACCEPT' => [Mime::JS, Mime::HTML, Mime::XML, "text/xml", Mime::ALL].join(",") ) - request.expects(:parameters).at_least_once.returns({}) - assert request.xhr? - assert_equal Mime::JS, request.format + + assert_called(request, :parameters, times: 1, returns: {}) do + assert request.xhr? + assert_equal Mime::JS, request.format + end end test "can override format with parameter negative" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => :txt }) - assert !request.format.xml? + assert_called(request, :parameters, times: 2, returns: {format: :txt}) do + assert !request.format.xml? + end end test "can override format with parameter positive" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => :xml }) - assert request.format.xml? + assert_called(request, :parameters, times: 2, returns: {format: :xml}) do + assert request.format.xml? + end end test "formats text/html with accept header" do @@ -810,23 +817,26 @@ class RequestFormat < BaseRequestTest test "formats format:text with accept header" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => :txt }) - assert_equal [Mime::TEXT], request.formats + assert_called(request, :parameters, times: 2, returns: {format: :txt}) do + assert_equal [Mime::TEXT], request.formats + end end test "formats format:unknown with accept header" do request = stub_request - request.expects(:parameters).at_least_once.returns({ :format => :unknown }) - assert_instance_of Mime::NullType, request.format + assert_called(request, :parameters, times: 2, returns: {format: :unknown}) do + assert_instance_of Mime::NullType, request.format + end end test "format is not nil with unknown format" do request = stub_request - request.expects(:parameters).at_least_once.returns({ format: :hello }) - assert request.format.nil? - assert_not request.format.html? - assert_not request.format.xml? - assert_not request.format.json? + assert_called(request, :parameters, times: 2, returns: {format: :hello}) do + assert request.format.nil? + assert_not request.format.html? + assert_not request.format.xml? + assert_not request.format.json? + end end test "format does not throw exceptions when malformed parameters" do @@ -837,8 +847,9 @@ class RequestFormat < BaseRequestTest test "formats with xhr request" do request = stub_request 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" - request.expects(:parameters).at_least_once.returns({}) - assert_equal [Mime::JS], request.formats + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [Mime::JS], request.formats + end end test "ignore_accept_header" do @@ -847,30 +858,37 @@ class RequestFormat < BaseRequestTest begin request = stub_request 'HTTP_ACCEPT' => 'application/xml' - request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::HTML ], request.formats + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [ Mime::HTML ], request.formats + end request = stub_request 'HTTP_ACCEPT' => 'koz-asked/something-crazy' - request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::HTML ], request.formats + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [ Mime::HTML ], request.formats + end request = stub_request 'HTTP_ACCEPT' => '*/*;q=0.1' - request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::HTML ], request.formats + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [ Mime::HTML ], request.formats + end request = stub_request 'HTTP_ACCEPT' => 'application/jxw' - request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::HTML ], request.formats + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [ Mime::HTML ], request.formats + end request = stub_request 'HTTP_ACCEPT' => 'application/xml', 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" - request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::JS ], request.formats + + assert_called(request, :parameters, times: 1, returns: {}) do + assert_equal [ Mime::JS ], request.formats + end request = stub_request 'HTTP_ACCEPT' => 'application/xml', 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" - request.expects(:parameters).at_least_once.returns({:format => :json}) - assert_equal [ Mime::JSON ], request.formats + assert_called(request, :parameters, times: 2, returns: {format: :json}) do + assert_equal [ Mime::JSON ], request.formats + end ensure ActionDispatch::Request.ignore_accept_header = old_ignore_accept_header end @@ -922,12 +940,14 @@ end class RequestParameters < BaseRequestTest test "parameters" do request = stub_request - request.expects(:request_parameters).at_least_once.returns({ "foo" => 1 }) - request.expects(:query_parameters).at_least_once.returns({ "bar" => 2 }) - assert_equal({"foo" => 1, "bar" => 2}, request.parameters) - assert_equal({"foo" => 1}, request.request_parameters) - assert_equal({"bar" => 2}, request.query_parameters) + assert_called(request, :request_parameters, times: 2, returns: {"foo" => 1}) do + assert_called(request, :query_parameters, times: 2, returns: {"bar" => 2}) do + assert_equal({"foo" => 1, "bar" => 2}, request.parameters) + assert_equal({"foo" => 1}, request.request_parameters) + assert_equal({"bar" => 2}, request.query_parameters) + end + end end test "parameters not accessible after rack parse error" do diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 780e7dc3e2..5d74424de7 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -49,6 +49,11 @@ class ResponseTest < ActiveSupport::TestCase assert_equal 'utf-8', @response.charset end + def test_setting_content_type_header_impacts_content_type_method + @response.headers['Content-Type'] = "application/aaron" + assert_equal 'application/aaron', @response.content_type + end + test "simple output" do @response.body = "Hello, World!" @@ -67,6 +72,13 @@ class ResponseTest < ActiveSupport::TestCase assert_equal 200, ActionDispatch::Response.new('200 OK').status end + def test_only_set_charset_still_defaults_to_text_html + response = ActionDispatch::Response.new + response.charset = "utf-16" + _,headers,_ = response.to_a + assert_equal "text/html; charset=utf-16", headers['Content-Type'] + end + test "utf8 output" do @response.body = [1090, 1077, 1089, 1090].pack("U*") diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index 017e9ba2dd..7a5b8393dc 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -1,230 +1,199 @@ require 'abstract_unit' class SSLTest < ActionDispatch::IntegrationTest - def default_app - lambda { |env| - headers = {'Content-Type' => "text/html"} - headers['Set-Cookie'] = "id=1; path=/\ntoken=abc; path=/; secure; HttpOnly" - [200, headers, ["OK"]] + HEADERS = Rack::Utils::HeaderHash.new 'Content-Type' => 'text/html' + + attr_accessor :app + + def build_app(headers: {}, ssl_options: {}) + headers = HEADERS.merge(headers) + ActionDispatch::SSL.new lambda { |env| [200, headers, []] }, ssl_options + end +end + +class RedirectSSLTest < SSLTest + def assert_not_redirected(url, headers: {}) + self.app = build_app + get url, headers: headers + assert_response :ok + end + + def assert_redirected(host: nil, port: nil, status: 301, body: [], + deprecated_host: nil, deprecated_port: nil, + from: 'http://a/b?c=d', to: from.sub('http', 'https')) + + self.app = build_app ssl_options: { + redirect: { host: host, port: port, status: status, body: body }, + host: deprecated_host, port: deprecated_port } + + get from + assert_response status + assert_redirected_to to + assert_equal body.join, @response.body end - def app - @app ||= ActionDispatch::SSL.new(default_app) + test 'https is not redirected' do + assert_not_redirected 'https://example.org' end - attr_writer :app - def test_allows_https_url - get "https://example.org/path?key=value" - assert_response :success + test 'proxied https is not redirected' do + assert_not_redirected 'http://example.org', headers: { 'HTTP_X_FORWARDED_PROTO' => 'https' } end - def test_allows_https_proxy_header_url - get "http://example.org/", headers: { 'HTTP_X_FORWARDED_PROTO' => "https" } - assert_response :success + test 'http is redirected to https' do + assert_redirected end - def test_redirects_http_to_https - get "http://example.org/path?key=value" - assert_response :redirect - assert_equal "https://example.org/path?key=value", - response.headers['Location'] + test 'redirect with non-301 status' do + assert_redirected status: 307 end - def test_hsts_header_by_default - get "https://example.org/" - assert_equal "max-age=31536000", - response.headers['Strict-Transport-Security'] + test 'redirect with custom body' do + assert_redirected body: ['foo'] end - def test_no_hsts_with_insecure_connection - get "http://example.org/" - assert_not response.headers['Strict-Transport-Security'] + test 'redirect to specific host' do + assert_redirected host: 'ssl', to: 'https://ssl/b?c=d' end - def test_hsts_header - self.app = ActionDispatch::SSL.new(default_app, :hsts => true) - get "https://example.org/" - assert_equal "max-age=31536000", - response.headers['Strict-Transport-Security'] + test 'redirect to default port' do + assert_redirected port: 443 end - def test_disable_hsts_header - self.app = ActionDispatch::SSL.new(default_app, :hsts => false) - get "https://example.org/" - assert_not response.headers['Strict-Transport-Security'] + test 'redirect to non-default port' do + assert_redirected port: 8443, to: 'https://a:8443/b?c=d' end - def test_hsts_expires - self.app = ActionDispatch::SSL.new(default_app, :hsts => { :expires => 500 }) - get "https://example.org/" - assert_equal "max-age=500", - response.headers['Strict-Transport-Security'] + test 'redirect to different host and non-default port' do + assert_redirected host: 'ssl', port: 8443, to: 'https://ssl:8443/b?c=d' end - def test_hsts_expires_with_duration - self.app = ActionDispatch::SSL.new(default_app, :hsts => { :expires => 1.year }) - get "https://example.org/" - assert_equal "max-age=31557600", - response.headers['Strict-Transport-Security'] + test 'redirect to different host including port' do + assert_redirected host: 'ssl:443', to: 'https://ssl:443/b?c=d' end - def test_hsts_include_subdomains - self.app = ActionDispatch::SSL.new(default_app, :hsts => { :subdomains => true }) - get "https://example.org/" - assert_equal "max-age=31536000; includeSubDomains", - response.headers['Strict-Transport-Security'] + test ':host is deprecated, moved within redirect: { host: … }' do + assert_deprecated do + assert_redirected deprecated_host: 'foo', to: 'https://foo/b?c=d' + end end - def test_flag_cookies_as_secure - get "https://example.org/" - assert_equal ["id=1; path=/; secure", "token=abc; path=/; secure; HttpOnly" ], - response.headers['Set-Cookie'].split("\n") + test ':port is deprecated, moved within redirect: { port: … }' do + assert_deprecated do + assert_redirected deprecated_port: 1, to: 'https://a:1/b?c=d' + end end +end - def test_flag_cookies_as_secure_at_end_of_line - self.app = ActionDispatch::SSL.new(lambda { |env| - headers = { - 'Content-Type' => "text/html", - 'Set-Cookie' => "problem=def; path=/; HttpOnly; secure" - } - [200, headers, ["OK"]] - }) +class StrictTransportSecurityTest < SSLTest + EXPECTED = 'max-age=15552000' - get "https://example.org/" - assert_equal ["problem=def; path=/; HttpOnly; secure"], - response.headers['Set-Cookie'].split("\n") + def assert_hsts(expected, url: 'https://example.org', hsts: {}, headers: {}) + self.app = build_app ssl_options: { hsts: hsts }, headers: headers + get url + assert_equal expected, response.headers['Strict-Transport-Security'] end - def test_flag_cookies_as_secure_with_more_spaces_before - self.app = ActionDispatch::SSL.new(lambda { |env| - headers = { - 'Content-Type' => "text/html", - 'Set-Cookie' => "problem=def; path=/; HttpOnly; secure" - } - [200, headers, ["OK"]] - }) + test 'enabled by default' do + assert_hsts EXPECTED + end - get "https://example.org/" - assert_equal ["problem=def; path=/; HttpOnly; secure"], - response.headers['Set-Cookie'].split("\n") + test 'not sent with http:// responses' do + assert_hsts nil, url: 'http://example.org' end - def test_flag_cookies_as_secure_with_more_spaces_after - self.app = ActionDispatch::SSL.new(lambda { |env| - headers = { - 'Content-Type' => "text/html", - 'Set-Cookie' => "problem=def; path=/; secure; HttpOnly" - } - [200, headers, ["OK"]] - }) + test 'defers to app-provided header' do + assert_hsts 'app-provided', headers: { 'Strict-Transport-Security' => 'app-provided' } + end - get "https://example.org/" - assert_equal ["problem=def; path=/; secure; HttpOnly"], - response.headers['Set-Cookie'].split("\n") + test 'hsts: true enables default settings' do + assert_hsts EXPECTED, hsts: true end + test 'hsts: false sets max-age to zero, clearing browser HSTS settings' do + assert_hsts 'max-age=0', hsts: false + end - def test_flag_cookies_as_secure_with_has_not_spaces_before - self.app = ActionDispatch::SSL.new(lambda { |env| - headers = { - 'Content-Type' => "text/html", - 'Set-Cookie' => "problem=def; path=/;secure; HttpOnly" - } - [200, headers, ["OK"]] - }) + test ':expires sets max-age' do + assert_hsts 'max-age=500', hsts: { expires: 500 } + end - get "https://example.org/" - assert_equal ["problem=def; path=/;secure; HttpOnly"], - response.headers['Set-Cookie'].split("\n") + test ':expires supports AS::Duration arguments' do + assert_hsts 'max-age=31557600', hsts: { expires: 1.year } end - def test_flag_cookies_as_secure_with_has_not_spaces_after - self.app = ActionDispatch::SSL.new(lambda { |env| - headers = { - 'Content-Type' => "text/html", - 'Set-Cookie' => "problem=def; path=/; secure;HttpOnly" - } - [200, headers, ["OK"]] - }) + test 'include subdomains' do + assert_hsts "#{EXPECTED}; includeSubDomains", hsts: { subdomains: true } + end - get "https://example.org/" - assert_equal ["problem=def; path=/; secure;HttpOnly"], - response.headers['Set-Cookie'].split("\n") + test 'exclude subdomains' do + assert_hsts EXPECTED, hsts: { subdomains: false } end - def test_flag_cookies_as_secure_with_ignore_case - self.app = ActionDispatch::SSL.new(lambda { |env| - headers = { - 'Content-Type' => "text/html", - 'Set-Cookie' => "problem=def; path=/; Secure; HttpOnly" - } - [200, headers, ["OK"]] - }) + test 'opt in to browser preload lists' do + assert_hsts "#{EXPECTED}; preload", hsts: { preload: true } + end - get "https://example.org/" - assert_equal ["problem=def; path=/; Secure; HttpOnly"], - response.headers['Set-Cookie'].split("\n") + test 'opt out of browser preload lists' do + assert_hsts EXPECTED, hsts: { preload: false } end +end - def test_no_cookies - self.app = ActionDispatch::SSL.new(lambda { |env| - [200, {'Content-Type' => "text/html"}, ["OK"]] - }) - get "https://example.org/" - assert !response.headers['Set-Cookie'] +class SecureCookiesTest < SSLTest + DEFAULT = %(id=1; path=/\ntoken=abc; path=/; secure; HttpOnly) + + def get(**options) + self.app = build_app(**options) + super 'https://example.org' + end + + def assert_cookies(*expected) + assert_equal expected, response.headers['Set-Cookie'].split("\n") + end + + def test_flag_cookies_as_secure + get headers: { 'Set-Cookie' => DEFAULT } + assert_cookies 'id=1; path=/; secure', 'token=abc; path=/; secure; HttpOnly' end - def test_redirect_to_host - self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org") - get "http://example.org/path?key=value" - assert_equal "https://ssl.example.org/path?key=value", - response.headers['Location'] + def test_flag_cookies_as_secure_at_end_of_line + get headers: { 'Set-Cookie' => 'problem=def; path=/; HttpOnly; secure' } + assert_cookies 'problem=def; path=/; HttpOnly; secure' + end + + def test_flag_cookies_as_secure_with_more_spaces_before + get headers: { 'Set-Cookie' => 'problem=def; path=/; HttpOnly; secure' } + assert_cookies 'problem=def; path=/; HttpOnly; secure' end - def test_redirect_to_port - self.app = ActionDispatch::SSL.new(default_app, :port => 8443) - get "http://example.org/path?key=value" - assert_equal "https://example.org:8443/path?key=value", - response.headers['Location'] + def test_flag_cookies_as_secure_with_more_spaces_after + get headers: { 'Set-Cookie' => 'problem=def; path=/; secure; HttpOnly' } + assert_cookies 'problem=def; path=/; secure; HttpOnly' end - def test_redirect_to_host_and_port - self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org", :port => 8443) - get "http://example.org/path?key=value" - assert_equal "https://ssl.example.org:8443/path?key=value", - response.headers['Location'] + def test_flag_cookies_as_secure_with_has_not_spaces_before + get headers: { 'Set-Cookie' => 'problem=def; path=/;secure; HttpOnly' } + assert_cookies 'problem=def; path=/;secure; HttpOnly' end - def test_redirect_to_host_with_port - self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org:443") - get "http://example.org/path?key=value" - assert_equal "https://ssl.example.org:443/path?key=value", - response.headers['Location'] + def test_flag_cookies_as_secure_with_has_not_spaces_after + get headers: { 'Set-Cookie' => 'problem=def; path=/; secure;HttpOnly' } + assert_cookies 'problem=def; path=/; secure;HttpOnly' end - def test_redirect_to_secure_host_when_on_subdomain - self.app = ActionDispatch::SSL.new(default_app, :host => "ssl.example.org") - get "http://ssl.example.org/path?key=value" - assert_equal "https://ssl.example.org/path?key=value", - response.headers['Location'] + def test_flag_cookies_as_secure_with_ignore_case + get headers: { 'Set-Cookie' => 'problem=def; path=/; Secure; HttpOnly' } + assert_cookies 'problem=def; path=/; Secure; HttpOnly' end - def test_redirect_to_secure_subdomain_when_on_deep_subdomain - self.app = ActionDispatch::SSL.new(default_app, :host => "example.co.uk") - get "http://double.rainbow.what.does.it.mean.example.co.uk/path?key=value" - assert_equal "https://example.co.uk/path?key=value", - response.headers['Location'] + def test_no_cookies + get + assert_nil response.headers['Set-Cookie'] end def test_keeps_original_headers_behavior - headers = Rack::Utils::HeaderHash.new( - "Content-Type" => "text/html", - "Connection" => ["close"] - ) - self.app = ActionDispatch::SSL.new(lambda { |env| [200, headers, ["OK"]] }) - - get "https://example.org/" - assert_equal "close", response.headers["Connection"] + get headers: { 'Connection' => %w[close] } + assert_equal 'close', response.headers['Connection'] end end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index d512dae4e7..6a3af32946 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -1,4 +1,3 @@ -# encoding: UTF-8 require 'abstract_unit' module ActionDispatch |