aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md39
-rw-r--r--actionpack/lib/action_controller.rb1
-rw-r--r--actionpack/lib/action_controller/base.rb1
-rw-r--r--actionpack/lib/action_controller/metal/etag_with_flash.rb16
-rw-r--r--actionpack/lib/action_controller/metal/implicit_render.rb2
-rw-r--r--actionpack/lib/action_controller/test_case.rb27
-rw-r--r--actionpack/lib/action_dispatch/http/url.rb86
-rw-r--r--actionpack/lib/action_dispatch/middleware/ssl.rb10
-rw-r--r--actionpack/lib/action_dispatch/routing/mapper.rb232
-rw-r--r--actionpack/test/controller/flash_test.rb29
-rw-r--r--actionpack/test/controller/test_case_test.rb9
-rw-r--r--actionpack/test/dispatch/routing_test.rb18
-rw-r--r--actionpack/test/dispatch/ssl_test.rb14
13 files changed, 294 insertions, 190 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 1fb7e20417..a66a1e8af3 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,42 @@
+* Include the content of the flash in the auto-generated etag. This solves the following problem:
+
+ 1. POST /messages
+ 2. redirect_to messages_url, notice: 'Message was created'
+ 3. GET /messages/1
+ 4. GET /messages
+
+ Step 4 would before still include the flash message, even though it's no longer relevant,
+ because the etag cache was recorded with the flash in place and didn't change when it was gone.
+
+ *DHH*
+
+* SSL: Changes redirect behavior for all non-GET and non-HEAD requests
+ (like POST/PUT/PATCH etc) to `http://` resources to redirect to `https://`
+ with a [307 status code](http://tools.ietf.org/html/rfc7231#section-6.4.7) instead of [301 status code](http://tools.ietf.org/html/rfc7231#section-6.4.2).
+
+ 307 status code instructs the HTTP clients to preserve the original
+ request method while redirecting. It has been part of HTTP RFC since
+ 1999 and is implemented/recognized by most (if not all) user agents.
+
+ # Before
+ POST http://example.com/articles (i.e. ArticlesContoller#create)
+ redirects to
+ GET https://example.com/articles (i.e. ArticlesContoller#index)
+
+ # After
+ POST http://example.com/articles (i.e. ArticlesContoller#create)
+ redirects to
+ POST https://example.com/articles (i.e. ArticlesContoller#create)
+
+ *Chirag Singhal*
+
+* Add `:as` option to `ActionController:TestCase#process` and related methods.
+
+ Specifying `as: mime_type` allows the `CONTENT_TYPE` header to be specified
+ in controller tests without manually doing this through `@request.headers['CONTENT_TYPE']`.
+
+ *Everest Stefan Munro-Zeisberger*
+
* Show cache hits and misses when rendering partials.
Partials using the `cache` helper will show whether a render hit or missed
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index fc86a907b3..50f20aa789 100644
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -23,6 +23,7 @@ module ActionController
autoload :Cookies
autoload :DataStreaming
autoload :EtagWithTemplateDigest
+ autoload :EtagWithFlash
autoload :Flash
autoload :ForceSSL
autoload :Head
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 68a526eccb..ca8066cd82 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -213,6 +213,7 @@ module ActionController
Renderers::All,
ConditionalGet,
EtagWithTemplateDigest,
+ EtagWithFlash,
Caching,
MimeResponds,
ImplicitRender,
diff --git a/actionpack/lib/action_controller/metal/etag_with_flash.rb b/actionpack/lib/action_controller/metal/etag_with_flash.rb
new file mode 100644
index 0000000000..474d75f02e
--- /dev/null
+++ b/actionpack/lib/action_controller/metal/etag_with_flash.rb
@@ -0,0 +1,16 @@
+module ActionController
+ # When you're using the flash, it's generally used as a conditional on the view.
+ # This means the content of the view depends on the flash. Which in turn means
+ # that the etag for a response should be computed with the content of the flash
+ # in mind. This does that by including the content of the flash as a component
+ # in the etag that's generated for a response.
+ module EtagWithFlash
+ extend ActiveSupport::Concern
+
+ include ActionController::ConditionalGet
+
+ included do
+ etag { flash unless flash.empty? }
+ end
+ end
+end
diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb
index c414527d63..4137db99ea 100644
--- a/actionpack/lib/action_controller/metal/implicit_render.rb
+++ b/actionpack/lib/action_controller/metal/implicit_render.rb
@@ -48,7 +48,7 @@ module ActionController
"NOTE! For XHR/Ajax or API requests, this action would normally " \
"respond with 204 No Content: an empty white screen. Since you're " \
"loading it in a web browser, we assume that you expected to " \
- "actually render a template, not… nothing, so we're showing an " \
+ "actually render a template, not nothing, so we're showing an " \
"error to be extra-clear. If you expect 204 No Content, carry on. " \
"That's what you'll get from an XHR or API request. Give it a shot."
diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index 3a6bc92b3c..11a6cf8e15 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -7,7 +7,6 @@ require "action_controller/template_assertions"
require "rails-dom-testing"
module ActionController
- # :stopdoc:
class Metal
include Testing::Functional
end
@@ -211,10 +210,18 @@ module ActionController
end
# Superclass for ActionController functional tests. Functional tests allow you to
- # test a single controller action per test method. This should not be confused with
- # integration tests (see ActionDispatch::IntegrationTest), which are more like
- # "stories" that can involve multiple controllers and multiple actions (i.e. multiple
- # different HTTP requests).
+ # test a single controller action per test method.
+ #
+ # == Use integration style controller tests over functional style controller tests.
+ #
+ # Rails discourages the use of functional tests in favor of integration tests
+ # (use ActionDispatch::IntegrationTest).
+ #
+ # New Rails applications no longer generate functional style controller tests and they should
+ # only be used for backward compatibility. Integration style controller tests perform actual
+ # requests, whereas functional style controller tests merely simulate a request. Besides,
+ # integration tests are as fast as functional tests and provide lot of helpers such as +as+,
+ # +parsed_body+ for effective testing of controller actions including even API endpoints.
#
# == Basic example
#
@@ -442,6 +449,8 @@ module ActionController
# - +session+: A hash of parameters to store in the session. This may be +nil+.
# - +flash+: A hash of parameters to store in the flash. This may be +nil+.
# - +format+: Request format. Defaults to +nil+. Can be string or symbol.
+ # - +as+: Content type. Defaults to +nil+. Must be a symbol that corresponds
+ # to a mime type.
#
# Example calling +create+ action and sending two params:
#
@@ -462,7 +471,7 @@ module ActionController
check_required_ivars
if kwarg_request?(args)
- parameters, session, body, flash, http_method, format, xhr = args[0].values_at(:params, :session, :body, :flash, :method, :format, :xhr)
+ parameters, session, body, flash, http_method, format, xhr, as = args[0].values_at(:params, :session, :body, :flash, :method, :format, :xhr, :as)
else
http_method, parameters, session, flash = args
format = nil
@@ -507,6 +516,11 @@ module ActionController
@request.set_header "REQUEST_METHOD", http_method
+ if as
+ @request.content_type = Mime[as].to_s
+ format ||= as
+ end
+
parameters = parameters.symbolize_keys
generated_extras = @routes.generate_extras(parameters.merge(controller: controller_class_name, action: action.to_s))
@@ -675,5 +689,4 @@ module ActionController
include Behavior
end
- # :startdoc:
end
diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index e85ea90b94..06ffa983d1 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -192,11 +192,7 @@ module ActionDispatch
# Returns the complete URL used for this request.
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com'
# req.url # => "http://example.com"
def url
protocol + host_with_port + fullpath
@@ -204,14 +200,10 @@ module ActionDispatch
# Returns 'https://' if this is an SSL request and 'http://' otherwise.
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com'
# req.protocol # => "http://"
#
- # req = Request.new 'HTTP_HOST' => 'example.com', 'HTTPS' => 'on'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com', 'HTTPS' => 'on'
# req.protocol # => "https://"
def protocol
@protocol ||= ssl? ? "https://" : "http://"
@@ -219,17 +211,13 @@ module ActionDispatch
# Returns the \host and port for this request, such as "example.com:8080".
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com'
# req.raw_host_with_port # => "example.com"
#
- # req = Request.new 'HTTP_HOST' => 'example.com:80'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:80'
# req.raw_host_with_port # => "example.com:80"
#
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.raw_host_with_port # => "example.com:8080"
def raw_host_with_port
if forwarded = x_forwarded_host.presence
@@ -241,11 +229,7 @@ module ActionDispatch
# Returns the host for this request, such as "example.com".
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.host # => "example.com"
def host
raw_host_with_port.sub(/:\d+$/, "".freeze)
@@ -255,17 +239,13 @@ module ActionDispatch
# "example.com:8080". Port is only included if it is not a default port
# (80 or 443)
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com'
# req.host_with_port # => "example.com"
#
- # req = Request.new 'HTTP_HOST' => 'example.com:80'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:80'
# req.host_with_port # => "example.com"
#
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.host_with_port # => "example.com:8080"
def host_with_port
"#{host}#{port_string}"
@@ -273,14 +253,10 @@ module ActionDispatch
# Returns the port number of this request as an integer.
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com'
# req.port # => 80
#
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.port # => 8080
def port
@port ||= begin
@@ -294,11 +270,7 @@ module ActionDispatch
# Returns the standard \port number for this request's protocol.
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.standard_port # => 80
def standard_port
case protocol
@@ -309,14 +281,10 @@ module ActionDispatch
# Returns whether this request is using the standard port
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com:80'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:80'
# req.standard_port? # => true
#
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.standard_port? # => false
def standard_port?
port == standard_port
@@ -325,14 +293,10 @@ module ActionDispatch
# Returns a number \port suffix like 8080 if the \port number of this request
# is not the default HTTP \port 80 or HTTPS \port 443.
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com:80'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:80'
# req.optional_port # => nil
#
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.optional_port # => 8080
def optional_port
standard_port? ? nil : port
@@ -341,14 +305,10 @@ module ActionDispatch
# Returns a string \port suffix, including colon, like ":8080" if the \port
# number of this request is not the default HTTP \port 80 or HTTPS \port 443.
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'HTTP_HOST' => 'example.com:80'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:80'
# req.port_string # => ""
#
- # req = Request.new 'HTTP_HOST' => 'example.com:8080'
+ # req = ActionDispatch::Request.new 'HTTP_HOST' => 'example.com:8080'
# req.port_string # => ":8080"
def port_string
standard_port? ? "" : ":#{port}"
@@ -356,14 +316,10 @@ module ActionDispatch
# Returns the requested port, such as 8080, based on SERVER_PORT
#
- # class Request < Rack::Request
- # include ActionDispatch::Http::URL
- # end
- #
- # req = Request.new 'SERVER_PORT' => '80'
+ # req = ActionDispatch::Request.new 'SERVER_PORT' => '80'
# req.server_port # => 80
#
- # req = Request.new 'SERVER_PORT' => '8080'
+ # req = ActionDispatch::Request.new 'SERVER_PORT' => '8080'
# req.server_port # => 8080
def server_port
get_header("SERVER_PORT").to_i
diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb
index 0b81d0ad43..992daab3aa 100644
--- a/actionpack/lib/action_dispatch/middleware/ssl.rb
+++ b/actionpack/lib/action_dispatch/middleware/ssl.rb
@@ -133,12 +133,20 @@ module ActionDispatch
end
def redirect_to_https(request)
- [ @redirect.fetch(:status, 301),
+ [ @redirect.fetch(:status, redirection_status(request)),
{ "Content-Type" => "text/html",
"Location" => https_location_for(request) },
@redirect.fetch(:body, []) ]
end
+ def redirection_status(request)
+ if request.get? || request.head?
+ 301 # Issue a permanent redirect via a GET request.
+ else
+ 307 # Issue a fresh request redirect to preserve the HTTP method.
+ end
+ end
+
def https_location_for(request)
host = @redirect[:host] || request.host
port = @redirect[:port] || request.port
diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index 3f4f920a87..b3acac42fc 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -716,11 +716,7 @@ module ActionDispatch
def map_method(method, args, &block)
options = args.extract_options!
options[:via] = method
- if options.key?(:defaults)
- defaults(options.delete(:defaults)) { match(*args, options, &block) }
- else
- match(*args, options, &block)
- end
+ match(*args, options, &block)
self
end
end
@@ -1557,7 +1553,7 @@ module ActionDispatch
# match 'path' => 'controller#action', via: patch
# match 'path', to: 'controller#action', via: :post
# match 'path', 'otherpath', on: :member, via: :get
- def match(path, *rest)
+ def match(path, *rest, &block)
if rest.empty? && Hash === path
options = path
path, to = options.find { |name, _value| name.is_a?(String) }
@@ -1588,114 +1584,13 @@ module ActionDispatch
paths = [path] + rest
end
- if options[:on] && !VALID_ON_OPTIONS.include?(options[:on])
- raise ArgumentError, "Unknown scope #{on.inspect} given to :on"
- end
-
- if @scope[:to]
- options[:to] ||= @scope[:to]
- end
-
- if @scope[:controller] && @scope[:action]
- options[:to] ||= "#{@scope[:controller]}##{@scope[:action]}"
- end
-
- controller = options.delete(:controller) || @scope[:controller]
- option_path = options.delete :path
- to = options.delete :to
- via = Mapping.check_via Array(options.delete(:via) {
- @scope[:via]
- })
- formatted = options.delete(:format) { @scope[:format] }
- anchor = options.delete(:anchor) { true }
- options_constraints = options.delete(:constraints) || {}
-
- path_types = paths.group_by(&:class)
- path_types.fetch(String, []).each do |_path|
- route_options = options.dup
- if _path && option_path
- ActiveSupport::Deprecation.warn <<-eowarn
-Specifying strings for both :path and the route path is deprecated. Change things like this:
-
- match #{_path.inspect}, :path => #{option_path.inspect}
-
-to this:
-
- match #{option_path.inspect}, :as => #{_path.inspect}, :action => #{path.inspect}
- eowarn
- route_options[:action] = _path
- route_options[:as] = _path
- _path = option_path
- end
- to = get_to_from_path(_path, to, route_options[:action])
- decomposed_match(_path, controller, route_options, _path, to, via, formatted, anchor, options_constraints)
- end
-
- path_types.fetch(Symbol, []).each do |action|
- route_options = options.dup
- decomposed_match(action, controller, route_options, option_path, to, via, formatted, anchor, options_constraints)
- end
-
- self
- end
-
- def get_to_from_path(path, to, action)
- return to if to || action
-
- path_without_format = path.sub(/\(\.:format\)$/, "")
- if using_match_shorthand?(path_without_format)
- path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1').tr("-", "_")
- else
- nil
- end
- end
-
- def using_match_shorthand?(path)
- path =~ %r{^/?[-\w]+/[-\w/]+$}
- end
-
- def decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) # :nodoc:
- if on = options.delete(:on)
- send(on) { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) }
+ if options.key?(:defaults)
+ defaults(options.delete(:defaults)) { map_match(paths, options, &block) }
else
- case @scope.scope_level
- when :resources
- nested { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) }
- when :resource
- member { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) }
- else
- add_route(path, controller, options, _path, to, via, formatted, anchor, options_constraints)
- end
+ map_match(paths, options, &block)
end
end
- def add_route(action, controller, options, _path, to, via, formatted, anchor, options_constraints) # :nodoc:
- path = path_for_action(action, _path)
- raise ArgumentError, "path is required" if path.blank?
-
- action = action.to_s
-
- default_action = options.delete(:action) || @scope[:action]
-
- if action =~ /^[\w\-\/]+$/
- default_action ||= action.tr("-", "_") unless action.include?("/")
- else
- action = nil
- end
-
- as = if !options.fetch(:as, true) # if it's set to nil or false
- options.delete(:as)
- else
- name_for_action(options.delete(:as), action)
- end
-
- path = Mapping.normalize_path URI.parser.escape(path), formatted
- ast = Journey::Parser.parse path
-
- mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options)
- @set.add_route(mapping, ast, as, anchor)
- end
-
# You can specify what Rails should route "/" to with the root method:
#
# root to: 'pages#main'
@@ -1912,6 +1807,7 @@ to this:
def api_only?
@set.api_only?
end
+
private
def path_scope(path)
@@ -1921,16 +1817,120 @@ to this:
@scope = @scope.parent
end
+ def map_match(paths, options)
+ if options[:on] && !VALID_ON_OPTIONS.include?(options[:on])
+ raise ArgumentError, "Unknown scope #{on.inspect} given to :on"
+ end
+
+ if @scope[:to]
+ options[:to] ||= @scope[:to]
+ end
+
+ if @scope[:controller] && @scope[:action]
+ options[:to] ||= "#{@scope[:controller]}##{@scope[:action]}"
+ end
+
+ controller = options.delete(:controller) || @scope[:controller]
+ option_path = options.delete :path
+ to = options.delete :to
+ via = Mapping.check_via Array(options.delete(:via) {
+ @scope[:via]
+ })
+ formatted = options.delete(:format) { @scope[:format] }
+ anchor = options.delete(:anchor) { true }
+ options_constraints = options.delete(:constraints) || {}
+
+ path_types = paths.group_by(&:class)
+ path_types.fetch(String, []).each do |_path|
+ route_options = options.dup
+ if _path && option_path
+ ActiveSupport::Deprecation.warn <<-eowarn
+Specifying strings for both :path and the route path is deprecated. Change things like this:
+
+ match #{_path.inspect}, :path => #{option_path.inspect}
+
+to this:
+
+ match #{option_path.inspect}, :as => #{_path.inspect}, :action => #{_path.inspect}
+ eowarn
+ route_options[:action] = _path
+ route_options[:as] = _path
+ _path = option_path
+ end
+ to = get_to_from_path(_path, to, route_options[:action])
+ decomposed_match(_path, controller, route_options, _path, to, via, formatted, anchor, options_constraints)
+ end
+
+ path_types.fetch(Symbol, []).each do |action|
+ route_options = options.dup
+ decomposed_match(action, controller, route_options, option_path, to, via, formatted, anchor, options_constraints)
+ end
+
+ self
+ end
+
+ def get_to_from_path(path, to, action)
+ return to if to || action
+
+ path_without_format = path.sub(/\(\.:format\)$/, "")
+ if using_match_shorthand?(path_without_format)
+ path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1').tr("-", "_")
+ else
+ nil
+ end
+ end
+
+ def using_match_shorthand?(path)
+ path =~ %r{^/?[-\w]+/[-\w/]+$}
+ end
+
+ def decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) # :nodoc:
+ if on = options.delete(:on)
+ send(on) { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) }
+ else
+ case @scope.scope_level
+ when :resources
+ nested { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) }
+ when :resource
+ member { decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) }
+ else
+ add_route(path, controller, options, _path, to, via, formatted, anchor, options_constraints)
+ end
+ end
+ end
+
+ def add_route(action, controller, options, _path, to, via, formatted, anchor, options_constraints) # :nodoc:
+ path = path_for_action(action, _path)
+ raise ArgumentError, "path is required" if path.blank?
+
+ action = action.to_s
+
+ default_action = options.delete(:action) || @scope[:action]
+
+ if action =~ /^[\w\-\/]+$/
+ default_action ||= action.tr("-", "_") unless action.include?("/")
+ else
+ action = nil
+ end
+
+ as = if !options.fetch(:as, true) # if it's set to nil or false
+ options.delete(:as)
+ else
+ name_for_action(options.delete(:as), action)
+ end
+
+ path = Mapping.normalize_path URI.parser.escape(path), formatted
+ ast = Journey::Parser.parse path
+
+ mapping = Mapping.build(@scope, @set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options)
+ @set.add_route(mapping, ast, as, anchor)
+ end
+
def match_root_route(options)
name = has_named_route?(:root) ? nil : :root
- defaults_option = options.delete(:defaults)
args = ["/", { as: name, via: :get }.merge!(options)]
- if defaults_option
- defaults(defaults_option) { match(*args) }
- else
- match(*args)
- end
+ match(*args)
end
end
diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb
index cabbe2d608..e5f24f1a3a 100644
--- a/actionpack/test/controller/flash_test.rb
+++ b/actionpack/test/controller/flash_test.rb
@@ -263,6 +263,13 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest
flash[:bar] = "for great justice"
head :ok
end
+
+ def set_flash_optionally
+ flash.now.notice = params[:flash]
+ if stale? etag: "abe"
+ render inline: "maybe flash"
+ end
+ end
end
def test_flash
@@ -310,6 +317,28 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest
end
end
+ def test_flash_factored_into_etag
+ with_test_route_set do
+ get "/set_flash_optionally"
+ no_flash_etag = response.etag
+
+ get "/set_flash_optionally", params: { flash: "hello!" }
+ hello_flash_etag = response.etag
+
+ assert_not_equal no_flash_etag, hello_flash_etag
+
+ get "/set_flash_optionally", params: { flash: "hello!" }
+ another_hello_flash_etag = response.etag
+
+ assert_equal another_hello_flash_etag, hello_flash_etag
+
+ get "/set_flash_optionally", params: { flash: "goodbye!" }
+ goodbye_flash_etag = response.etag
+
+ assert_not_equal another_hello_flash_etag, goodbye_flash_etag
+ end
+ end
+
private
# Overwrite get to send SessionSecret in env hash
diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb
index 2d78fc71a9..696794a3eb 100644
--- a/actionpack/test/controller/test_case_test.rb
+++ b/actionpack/test/controller/test_case_test.rb
@@ -637,6 +637,15 @@ XML
assert_equal "application/json", parsed_env["CONTENT_TYPE"]
end
+ def test_using_as_json_sets_request_content_type_to_json
+ post :render_body, params: { bool_value: true, str_value: "string", num_value: 2 }, as: :json
+
+ assert_equal "application/json", @request.headers["CONTENT_TYPE"]
+ assert_equal true, @request.request_parameters[:bool_value]
+ assert_equal "string", @request.request_parameters[:str_value]
+ assert_equal 2, @request.request_parameters[:num_value]
+ 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" }
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 56be08f54f..ee5c30ef0e 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -1759,6 +1759,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
assert_equal 1, @request.params[:page]
end
+ def test_keyed_default_string_params_with_match
+ draw do
+ match "/", to: "pages#show", via: :get, defaults: { id: "home" }
+ end
+
+ get "/"
+ assert_equal "home", @request.params[:id]
+ end
+
+ def test_default_string_params_with_match
+ draw do
+ match "/", to: "pages#show", via: :get, id: "home"
+ end
+
+ get "/"
+ assert_equal "home", @request.params[:id]
+ end
+
def test_keyed_default_string_params_with_root
draw do
root to: "pages#show", defaults: { id: "home" }
diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb
index ccddb90bb5..71b274bf1e 100644
--- a/actionpack/test/dispatch/ssl_test.rb
+++ b/actionpack/test/dispatch/ssl_test.rb
@@ -38,6 +38,16 @@ class RedirectSSLTest < SSLTest
assert_equal redirect[:body].join, @response.body
end
+ def assert_post_redirected(redirect: {}, from: "http://a/b?c=d",
+ to: from.sub("http", "https"))
+
+ self.app = build_app ssl_options: { redirect: redirect }
+
+ post from
+ assert_response redirect[:status] || 307
+ assert_redirected_to to
+ end
+
test "exclude can avoid redirect" do
excluding = { exclude: -> request { request.path =~ /healthcheck/ } }
@@ -57,6 +67,10 @@ class RedirectSSLTest < SSLTest
assert_redirected
end
+ test "http POST is redirected to https with status 307" do
+ assert_post_redirected
+ end
+
test "redirect with non-301 status" do
assert_redirected redirect: { status: 307 }
end