aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack
diff options
context:
space:
mode:
Diffstat (limited to 'actionpack')
-rw-r--r--actionpack/CHANGELOG.md10
-rw-r--r--actionpack/lib/action_controller/metal/basic_implicit_render.rb2
-rw-r--r--actionpack/lib/action_controller/metal/force_ssl.rb3
-rw-r--r--actionpack/lib/action_controller/metal/implicit_render.rb4
-rw-r--r--actionpack/lib/action_controller/metal/redirecting.rb33
-rw-r--r--actionpack/lib/action_dispatch/testing/assertions/routing.rb9
-rw-r--r--actionpack/test/controller/action_pack_assertions_test.rb6
-rw-r--r--actionpack/test/controller/log_subscriber_test.rb4
-rw-r--r--actionpack/test/controller/new_base/render_context_test.rb55
-rw-r--r--actionpack/test/controller/redirect_test.rb44
-rw-r--r--actionpack/test/dispatch/debug_exceptions_test.rb44
-rw-r--r--actionpack/test/dispatch/live_response_test.rb2
-rw-r--r--actionpack/test/journey/router_test.rb6
13 files changed, 57 insertions, 165 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 1a1b1034aa..1d2f6b09c3 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -11,12 +11,6 @@
*Rafael Mendonça França*
-* Ensure external redirects are explicitly allowed
-
- Add `fallback_location` and `allow_other_host` options to `redirect_to`.
-
- *Gannon McGibbon*
-
* Introduce ActionDispatch::HostAuthorization
This is a new middleware that guards against DNS rebinding attacks by
@@ -82,7 +76,7 @@
* Apply mapping to symbols returned from dynamic CSP sources
Previously if a dynamic source returned a symbol such as :self it
- would be converted to a string implicity, e.g:
+ would be converted to a string implicitly, e.g:
policy.default_src -> { :self }
@@ -135,7 +129,7 @@
*Assain Jaleel*
-* Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations.
+* Raises `ActionController::RespondToMismatchError` with conflicting `respond_to` invocations.
`respond_to` can match multiple types and lead to undefined behavior when
multiple invocations are made and the types do not match:
diff --git a/actionpack/lib/action_controller/metal/basic_implicit_render.rb b/actionpack/lib/action_controller/metal/basic_implicit_render.rb
index 2dc990f303..f9a758ff0e 100644
--- a/actionpack/lib/action_controller/metal/basic_implicit_render.rb
+++ b/actionpack/lib/action_controller/metal/basic_implicit_render.rb
@@ -6,7 +6,7 @@ module ActionController
super.tap { default_render unless performed? }
end
- def default_render(*args)
+ def default_render
head :no_content
end
end
diff --git a/actionpack/lib/action_controller/metal/force_ssl.rb b/actionpack/lib/action_controller/metal/force_ssl.rb
index 205f84ae36..93fd57b640 100644
--- a/actionpack/lib/action_controller/metal/force_ssl.rb
+++ b/actionpack/lib/action_controller/metal/force_ssl.rb
@@ -13,7 +13,7 @@ module ActionController
ACTION_OPTIONS = [:only, :except, :if, :unless]
URL_OPTIONS = [:protocol, :host, :domain, :subdomain, :port, :path]
- REDIRECT_OPTIONS = [:status, :flash, :alert, :notice, :allow_other_host]
+ REDIRECT_OPTIONS = [:status, :flash, :alert, :notice]
module ClassMethods # :nodoc:
def force_ssl(options = {})
@@ -41,7 +41,6 @@ module ActionController
host: request.host,
path: request.fullpath,
status: :moved_permanently,
- allow_other_host: true,
}
if host_or_options.is_a?(Hash)
diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb
index d3bb58f48b..8365ddca57 100644
--- a/actionpack/lib/action_controller/metal/implicit_render.rb
+++ b/actionpack/lib/action_controller/metal/implicit_render.rb
@@ -30,9 +30,9 @@ module ActionController
# :stopdoc:
include BasicImplicitRender
- def default_render(*args)
+ def default_render
if template_exists?(action_name.to_s, _prefixes, variants: request.variant)
- render(*args)
+ render
elsif any_templates?(action_name.to_s, _prefixes)
message = "#{self.class.name}\##{action_name} is missing a template " \
"for this request format and variant.\n" \
diff --git a/actionpack/lib/action_controller/metal/redirecting.rb b/actionpack/lib/action_controller/metal/redirecting.rb
index 8bd003f5ed..67c198d150 100644
--- a/actionpack/lib/action_controller/metal/redirecting.rb
+++ b/actionpack/lib/action_controller/metal/redirecting.rb
@@ -60,7 +60,7 @@ module ActionController
raise AbstractController::DoubleRenderError if response_body
self.status = _extract_redirect_to_status(options, response_options)
- self.location = _compute_safe_redirect_to_location(request, options, response_options)
+ self.location = _compute_redirect_to_location(request, options)
self.response_body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(response.location)}\">redirected</a>.</body></html>"
end
@@ -88,13 +88,9 @@ module ActionController
# All other options that can be passed to <tt>redirect_to</tt> are accepted as
# options and the behavior is identical.
def redirect_back(fallback_location:, allow_other_host: true, **args)
- referer = request.headers.fetch("Referer", fallback_location)
- response_options = {
- fallback_location: fallback_location,
- allow_other_host: allow_other_host,
- **args,
- }
- redirect_to referer, response_options
+ referer = request.headers["Referer"]
+ redirect_to_referer = referer && (allow_other_host || _url_host_allowed?(referer))
+ redirect_to redirect_to_referer ? referer : fallback_location, **args
end
def _compute_redirect_to_location(request, options) #:nodoc:
@@ -118,23 +114,6 @@ module ActionController
public :_compute_redirect_to_location
private
- def _compute_safe_redirect_to_location(request, options, response_options)
- location = _compute_redirect_to_location(request, options)
- location_options = options.is_a?(Hash) ? options : {}
- if response_options[:allow_other_host] || _url_host_allowed?(location, location_options)
- location
- else
- fallback_location = response_options.fetch(:fallback_location) do
- raise ArgumentError, <<~MSG.squish
- Unsafe redirect #{location.inspect},
- use :fallback_location to specify a fallback
- or :allow_other_host to redirect anyway.
- MSG
- end
- _compute_redirect_to_location(request, fallback_location)
- end
- end
-
def _extract_redirect_to_status(options, response_options)
if options.is_a?(Hash) && options.key?(:status)
Rack::Utils.status_code(options.delete(:status))
@@ -145,8 +124,8 @@ module ActionController
end
end
- def _url_host_allowed?(url, options = {})
- URI(url.to_s).host.in?([request.host, options[:host]])
+ def _url_host_allowed?(url)
+ URI(url.to_s).host == request.host
rescue ArgumentError, URI::Error
false
end
diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
index af41521c5c..28cde6704e 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb
@@ -160,9 +160,16 @@ module ActionDispatch
@controller.singleton_class.include(_routes.url_helpers)
if @controller.respond_to? :view_context_class
- @controller.view_context_class = Class.new(@controller.view_context_class) do
+ view_context_class = Class.new(@controller.view_context_class) do
include _routes.url_helpers
end
+
+ custom_view_context = Module.new {
+ define_method(:view_context_class) do
+ view_context_class
+ end
+ }
+ @controller.extend(custom_view_context)
end
end
yield @routes
diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb
index c7aae034dd..ecb8c37e6b 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -28,13 +28,13 @@ class ActionPackAssertionsController < ActionController::Base
def redirect_to_path() redirect_to "/some/path" end
- def redirect_invalid_external_route() redirect_to "ht_tp://www.rubyonrails.org", allow_other_host: true end
+ def redirect_invalid_external_route() redirect_to "ht_tp://www.rubyonrails.org" end
def redirect_to_named_route() redirect_to route_one_url end
- def redirect_external() redirect_to "http://www.rubyonrails.org", allow_other_host: true; end
+ def redirect_external() redirect_to "http://www.rubyonrails.org"; end
- def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org", allow_other_host: true; end
+ def redirect_external_protocol_relative() redirect_to "//www.rubyonrails.org"; end
def response404() head "404 AWOL" end
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index cbebc6b59c..0562c16284 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -25,11 +25,11 @@ module Another
end
def redirector
- redirect_to "http://foo.bar/", allow_other_host: true
+ redirect_to "http://foo.bar/"
end
def filterable_redirector
- redirect_to "http://secret.foo.bar/", allow_other_host: true
+ redirect_to "http://secret.foo.bar/"
end
def data_sender
diff --git a/actionpack/test/controller/new_base/render_context_test.rb b/actionpack/test/controller/new_base/render_context_test.rb
deleted file mode 100644
index 5e570a1d79..0000000000
--- a/actionpack/test/controller/new_base/render_context_test.rb
+++ /dev/null
@@ -1,55 +0,0 @@
-# frozen_string_literal: true
-
-require "abstract_unit"
-
-# This is testing the decoupling of view renderer and view context
-# by allowing the controller to be used as view context. This is
-# similar to the way sinatra renders templates.
-module RenderContext
- class BasicController < ActionController::Base
- self.view_paths = [ActionView::FixtureResolver.new(
- "render_context/basic/hello_world.html.erb" => "<%= @value %> from <%= self.__controller_method__ %>",
- "layouts/basic.html.erb" => "?<%= yield %>?"
- )]
-
- # 1) Include ActionView::Context to bring the required dependencies
- include ActionView::Context
-
- # 2) Call _prepare_context that will do the required initialization
- before_action :_prepare_context
-
- def hello_world
- @value = "Hello"
- render action: "hello_world", layout: false
- end
-
- def with_layout
- @value = "Hello"
- render action: "hello_world", layout: "basic"
- end
-
- protected def __controller_method__
- "controller context!"
- end
-
- private
- # 3) Set view_context to self
- def view_context
- self
- end
- end
-
- class RenderContextTest < Rack::TestCase
- test "rendering using the controller as context" do
- get "/render_context/basic/hello_world"
- assert_body "Hello from controller context!"
- assert_status 200
- end
-
- test "rendering using the controller as context with layout" do
- get "/render_context/basic/with_layout"
- assert_body "?Hello from controller context!?"
- assert_status 200
- end
- end
-end
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index 945d2275c0..998498e1b2 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -49,11 +49,11 @@ class RedirectController < ActionController::Base
end
def url_redirect_with_status
- redirect_to("http://www.example.com", status: :moved_permanently, allow_other_host: true)
+ redirect_to("http://www.example.com", status: :moved_permanently)
end
def url_redirect_with_status_hash
- redirect_to("http://www.example.com", status: 301, allow_other_host: true)
+ redirect_to("http://www.example.com", status: 301)
end
def relative_url_redirect_with_status
@@ -81,27 +81,19 @@ class RedirectController < ActionController::Base
end
def redirect_to_url
- redirect_to "http://www.rubyonrails.org/", allow_other_host: true
- end
-
- def redirect_to_unsafe_url
redirect_to "http://www.rubyonrails.org/"
end
- def redirect_to_relative_unsafe_url
- redirect_to ".br"
- end
-
def redirect_to_url_with_unescaped_query_string
- redirect_to "http://example.com/query?status=new", allow_other_host: true
+ redirect_to "http://example.com/query?status=new"
end
def redirect_to_url_with_complex_scheme
- redirect_to "x-test+scheme.complex:redirect", allow_other_host: true
+ redirect_to "x-test+scheme.complex:redirect"
end
def redirect_to_url_with_network_path_reference
- redirect_to "//www.rubyonrails.org/", allow_other_host: true
+ redirect_to "//www.rubyonrails.org/"
end
def redirect_to_existing_record
@@ -121,12 +113,12 @@ class RedirectController < ActionController::Base
end
def redirect_to_with_block
- redirect_to proc { "http://www.rubyonrails.org/" }, allow_other_host: true
+ redirect_to proc { "http://www.rubyonrails.org/" }
end
def redirect_to_with_block_and_assigns
@url = "http://www.rubyonrails.org/"
- redirect_to proc { @url }, allow_other_host: true
+ redirect_to proc { @url }
end
def redirect_to_with_block_and_options
@@ -253,28 +245,6 @@ class RedirectTest < ActionController::TestCase
assert_redirected_to "http://www.rubyonrails.org/"
end
- def test_redirect_to_unsafe_url
- error = assert_raises(ArgumentError) do
- get :redirect_to_unsafe_url
- end
- assert_equal <<~MSG.squish, error.message
- Unsafe redirect \"http://www.rubyonrails.org/\",
- use :fallback_location to specify a fallback or
- :allow_other_host to redirect anyway.
- MSG
- end
-
- def test_redirect_to_relative_unsafe_url
- error = assert_raises(ArgumentError) do
- get :redirect_to_relative_unsafe_url
- end
- assert_equal <<~MSG.squish, error.message
- Unsafe redirect \"http://test.host.br\",
- use :fallback_location to specify a fallback or
- :allow_other_host to redirect anyway.
- MSG
- end
-
def test_redirect_to_url_with_unescaped_query_string
get :redirect_to_url_with_unescaped_query_string
assert_response :redirect
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index c326f276bf..214e063276 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -8,7 +8,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
class Boomer
attr_accessor :closed
- def initialize(detailed = false)
+ def initialize(detailed = false)
@detailed = detailed
@closed = false
end
@@ -39,52 +39,50 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
def call(env)
env["action_dispatch.show_detailed_exceptions"] = @detailed
req = ActionDispatch::Request.new(env)
+ template = ActionView::Template.new(File.read(__FILE__), __FILE__, ActionView::Template::Handlers::Raw.new, {})
+
case req.path
- when %r{/pass}
+ when "/pass"
[404, { "X-Cascade" => "pass" }, self]
- when %r{/not_found}
+ when "/not_found"
raise AbstractController::ActionNotFound
- when %r{/runtime_error}
+ when "/runtime_error"
raise RuntimeError
- when %r{/method_not_allowed}
+ when "/method_not_allowed"
raise ActionController::MethodNotAllowed
- when %r{/intercepted_error}
+ when "/intercepted_error"
raise InterceptedErrorInstance
- when %r{/unknown_http_method}
+ when "/unknown_http_method"
raise ActionController::UnknownHttpMethod
- when %r{/not_implemented}
+ when "/not_implemented"
raise ActionController::NotImplemented
- when %r{/unprocessable_entity}
+ when "/unprocessable_entity"
raise ActionController::InvalidAuthenticityToken
- when %r{/not_found_original_exception}
+ when "/not_found_original_exception"
begin
raise AbstractController::ActionNotFound.new
rescue
- raise ActionView::Template::Error.new("template")
+ raise ActionView::Template::Error.new(template)
end
- when %r{/missing_template}
+ when "/missing_template"
raise ActionView::MissingTemplate.new(%w(foo), "foo/index", %w(foo), false, "mailer")
- when %r{/bad_request}
+ when "/bad_request"
raise ActionController::BadRequest
- when %r{/missing_keys}
+ when "/missing_keys"
raise ActionController::UrlGenerationError, "No route matches"
- when %r{/parameter_missing}
+ when "/parameter_missing"
raise ActionController::ParameterMissing, :missing_param_key
- when %r{/original_syntax_error}
+ when "/original_syntax_error"
eval "broke_syntax =" # `eval` need for raise native SyntaxError at runtime
- when %r{/syntax_error_into_view}
+ when "/syntax_error_into_view"
begin
eval "broke_syntax ="
rescue Exception
- template = ActionView::Template.new(File.read(__FILE__),
- __FILE__,
- ActionView::Template::Handlers::Raw.new,
- {})
raise ActionView::Template::Error.new(template)
end
- when %r{/framework_raises}
+ when "/framework_raises"
method_that_raises
- when %r{/nested_exceptions}
+ when "/nested_exceptions"
raise_nested_exceptions
else
raise "puke!"
diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb
index a9a56f205f..b673fd3805 100644
--- a/actionpack/test/dispatch/live_response_test.rb
+++ b/actionpack/test/dispatch/live_response_test.rb
@@ -62,7 +62,7 @@ module ActionController
assert_nil @response.headers["Content-Length"]
end
- def test_headers_cannot_be_written_after_webserver_reads
+ def test_headers_cannot_be_written_after_web_server_reads
@response.stream.write "omg"
latch = Concurrent::CountDownLatch.new
diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb
index 1f4e14aef6..f8d89def6a 100644
--- a/actionpack/test/journey/router_test.rb
+++ b/actionpack/test/journey/router_test.rb
@@ -284,7 +284,7 @@ module ActionDispatch
def test_generate_missing_keys_no_matches_different_format_keys
get "/:controller/:action/:name", to: "foo#bar"
- primarty_parameters = {
+ primary_parameters = {
id: 1,
controller: "tasks",
action: "show",
@@ -297,9 +297,9 @@ module ActionDispatch
missing_parameters = {
missing_key => "task_1"
}
- request_parameters = primarty_parameters.merge(redirection_parameters).merge(missing_parameters)
+ request_parameters = primary_parameters.merge(redirection_parameters).merge(missing_parameters)
- message = "No route matches #{Hash[request_parameters.sort_by { |k, v|k.to_s }].inspect}, missing required keys: #{[missing_key.to_sym].inspect}"
+ message = "No route matches #{Hash[request_parameters.sort_by { |k, _|k.to_s }].inspect}, missing required keys: #{[missing_key.to_sym].inspect}"
error = assert_raises(ActionController::UrlGenerationError) do
@formatter.generate(