10 files changed, 308 insertions, 160 deletions

diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb
index 13ad22b5c5..c8c885f35c 100644
--- a/actionpack/test/dispatch/content_security_policy_test.rb
+++ b/actionpack/test/dispatch/content_security_policy_test.rb
@@ -260,12 +260,13 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT
   ROUTES.draw do
     scope module: "default_content_security_policy_integration_test" do
       get "/", to: "policy#index"
+      get "/redirect", to: redirect("/")
     end
   end
 
   POLICY = ActionDispatch::ContentSecurityPolicy.new do |p|
-    p.default_src :self
-    p.script_src  :https
+    p.default_src -> { :self  }
+    p.script_src  -> { :https }
   end
 
   class PolicyConfigMiddleware
@@ -295,14 +296,19 @@ class DefaultContentSecurityPolicyIntegrationTest < ActionDispatch::IntegrationT
   def test_adds_nonce_to_script_src_content_security_policy_only_once
     get "/"
     get "/"
+    assert_response :success
+    assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='"
+  end
+
+  def test_redirect_works_with_dynamic_sources
+    get "/redirect"
+    assert_response :redirect
     assert_policy "default-src 'self'; script-src https: 'nonce-iyhD0Yc0W+c='"
   end
 
   private
 
     def assert_policy(expected, report_only: false)
-      assert_response :success
-
       if report_only
         expected_header = "Content-Security-Policy-Report-Only"
         unexpected_header = "Content-Security-Policy"
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb
index 37399cfd07..c326f276bf 100644
--- a/actionpack/test/dispatch/debug_exceptions_test.rb
+++ b/actionpack/test/dispatch/debug_exceptions_test.rb
@@ -27,14 +27,12 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
     end
 
     def raise_nested_exceptions
+      raise "First error"
+    rescue
       begin
-        raise "First error"
+        raise "Second error"
       rescue
-        begin
-          raise "Second error"
-        rescue
-          raise "Third error"
-        end
+        raise "Third error"
       end
     end
 
@@ -290,22 +288,20 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest
   end
 
   test "rescue with JSON format as fallback if API request format is not supported" do
-    begin
-      Mime::Type.register "text/wibble", :wibble
+    Mime::Type.register "text/wibble", :wibble
 
-      ActionDispatch::IntegrationTest.register_encoder(:wibble,
-        param_encoder: -> params { params })
+    ActionDispatch::IntegrationTest.register_encoder(:wibble,
+      param_encoder: -> params { params })
 
-      @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
+    @app = ActionDispatch::DebugExceptions.new(Boomer.new(true), RoutesApp, :api)
 
-      get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble
-      assert_response 500
-      assert_equal "application/json", response.content_type
-      assert_match(/RuntimeError: puke/, body)
+    get "/index", headers: { "action_dispatch.show_exceptions" => true }, as: :wibble
+    assert_response 500
+    assert_equal "application/json", response.content_type
+    assert_match(/RuntimeError: puke/, body)
 
-    ensure
-      Mime::Type.unregister :wibble
-    end
+  ensure
+    Mime::Type.unregister :wibble
   end
 
   test "does not show filtered parameters" do
diff --git a/actionpack/test/dispatch/host_authorization_test.rb b/actionpack/test/dispatch/host_authorization_test.rb
new file mode 100644
index 0000000000..dae7b08ec1
--- /dev/null
+++ b/actionpack/test/dispatch/host_authorization_test.rb
@@ -0,0 +1,161 @@
+# frozen_string_literal: true
+
+require "abstract_unit"
+require "ipaddr"
+
+class HostAuthorizationTest < ActionDispatch::IntegrationTest
+  App = -> env { [200, {}, %w(Success)] }
+
+  test "blocks requests to unallowed host" do
+    @app = ActionDispatch::HostAuthorization.new(App, %w(only.com))
+
+    get "/"
+
+    assert_response :forbidden
+    assert_match "Blocked host: www.example.com", response.body
+  end
+
+  test "passes all requests to if the whitelist is empty" do
+    @app = ActionDispatch::HostAuthorization.new(App, nil)
+
+    get "/"
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+
+  test "passes requests to allowed host" do
+    @app = ActionDispatch::HostAuthorization.new(App, %w(www.example.com))
+
+    get "/"
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+
+  test "the whitelist could be a single element" do
+    @app = ActionDispatch::HostAuthorization.new(App, "www.example.com")
+
+    get "/"
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+
+  test "passes requests to allowed hosts with domain name notation" do
+    @app = ActionDispatch::HostAuthorization.new(App, ".example.com")
+
+    get "/"
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+
+  test "does not allow domain name notation in the HOST header itself" do
+    @app = ActionDispatch::HostAuthorization.new(App, ".example.com")
+
+    get "/", env: {
+      "HOST" => ".example.com",
+    }
+
+    assert_response :forbidden
+    assert_match "Blocked host: .example.com", response.body
+  end
+
+  test "checks for requests with #=== to support wider range of host checks" do
+    @app = ActionDispatch::HostAuthorization.new(App, [-> input { input == "www.example.com" }])
+
+    get "/"
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+
+  test "mark the host when authorized" do
+    @app = ActionDispatch::HostAuthorization.new(App, ".example.com")
+
+    get "/"
+
+    assert_equal "www.example.com", request.get_header("action_dispatch.authorized_host")
+  end
+
+  test "sanitizes regular expressions to prevent accidental matches" do
+    @app = ActionDispatch::HostAuthorization.new(App, [/w.example.co/])
+
+    get "/"
+
+    assert_response :forbidden
+    assert_match "Blocked host: www.example.com", response.body
+  end
+
+  test "blocks requests to unallowed host supporting custom responses" do
+    @app = ActionDispatch::HostAuthorization.new(App, ["w.example.co"], -> env do
+      [401, {}, %w(Custom)]
+    end)
+
+    get "/"
+
+    assert_response :unauthorized
+    assert_equal "Custom", body
+  end
+
+  test "blocks requests with spoofed X-FORWARDED-HOST" do
+    @app = ActionDispatch::HostAuthorization.new(App, [IPAddr.new("127.0.0.1")])
+
+    get "/", env: {
+      "HTTP_X_FORWARDED_HOST" => "127.0.0.1",
+      "HOST" => "www.example.com",
+    }
+
+    assert_response :forbidden
+    assert_match "Blocked host: 127.0.0.1", response.body
+  end
+
+  test "does not consider IP addresses in X-FORWARDED-HOST spoofed when disabled" do
+    @app = ActionDispatch::HostAuthorization.new(App, nil)
+
+    get "/", env: {
+      "HTTP_X_FORWARDED_HOST" => "127.0.0.1",
+      "HOST" => "www.example.com",
+    }
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+
+  test "detects localhost domain spoofing" do
+    @app = ActionDispatch::HostAuthorization.new(App, "localhost")
+
+    get "/", env: {
+      "HTTP_X_FORWARDED_HOST" => "localhost",
+      "HOST" => "www.example.com",
+    }
+
+    assert_response :forbidden
+    assert_match "Blocked host: localhost", response.body
+  end
+
+  test "forwarded hosts should be permitted" do
+    @app = ActionDispatch::HostAuthorization.new(App, "domain.com")
+
+    get "/", env: {
+      "HTTP_X_FORWARDED_HOST" => "sub.domain.com",
+      "HOST" => "domain.com",
+    }
+
+    assert_response :forbidden
+    assert_match "Blocked host: sub.domain.com", response.body
+  end
+
+  test "forwarded hosts are allowed when permitted" do
+    @app = ActionDispatch::HostAuthorization.new(App, ".domain.com")
+
+    get "/", env: {
+      "HTTP_X_FORWARDED_HOST" => "sub.domain.com",
+      "HOST" => "domain.com",
+    }
+
+    assert_response :ok
+    assert_equal "Success", body
+  end
+end
diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb
index fa264417e1..45d91883c0 100644
--- a/actionpack/test/dispatch/mime_type_test.rb
+++ b/actionpack/test/dispatch/mime_type_test.rb
@@ -96,57 +96,47 @@ class MimeTypeTest < ActiveSupport::TestCase
   end
 
   test "custom type" do
-    begin
-      type = Mime::Type.register("image/foo", :foo)
-      assert_equal type, Mime[:foo]
-    ensure
-      Mime::Type.unregister(:foo)
-    end
+    type = Mime::Type.register("image/foo", :foo)
+    assert_equal type, Mime[:foo]
+  ensure
+    Mime::Type.unregister(:foo)
   end
 
   test "custom type with type aliases" do
-    begin
-      Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"]
-      %w[text/foobar text/foo text/bar].each do |type|
-        assert_equal Mime[:foobar], type
-      end
-    ensure
-      Mime::Type.unregister(:foobar)
+    Mime::Type.register "text/foobar", :foobar, ["text/foo", "text/bar"]
+    %w[text/foobar text/foo text/bar].each do |type|
+      assert_equal Mime[:foobar], type
     end
+  ensure
+    Mime::Type.unregister(:foobar)
   end
 
   test "register callbacks" do
-    begin
-      registered_mimes = []
-      Mime::Type.register_callback do |mime|
-        registered_mimes << mime
-      end
-
-      mime = Mime::Type.register("text/foo", :foo)
-      assert_equal [mime], registered_mimes
-    ensure
-      Mime::Type.unregister(:foo)
+    registered_mimes = []
+    Mime::Type.register_callback do |mime|
+      registered_mimes << mime
     end
+
+    mime = Mime::Type.register("text/foo", :foo)
+    assert_equal [mime], registered_mimes
+  ensure
+    Mime::Type.unregister(:foo)
   end
 
   test "custom type with extension aliases" do
-    begin
-      Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"]
-      %w[foobar foo bar].each do |extension|
-        assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension]
-      end
-    ensure
-      Mime::Type.unregister(:foobar)
+    Mime::Type.register "text/foobar", :foobar, [], [:foo, "bar"]
+    %w[foobar foo bar].each do |extension|
+      assert_equal Mime[:foobar], Mime::EXTENSION_LOOKUP[extension]
     end
+  ensure
+    Mime::Type.unregister(:foobar)
   end
 
   test "register alias" do
-    begin
-      Mime::Type.register_alias "application/xhtml+xml", :foobar
-      assert_equal Mime[:html], Mime::EXTENSION_LOOKUP["foobar"]
-    ensure
-      Mime::Type.unregister(:foobar)
-    end
+    Mime::Type.register_alias "application/xhtml+xml", :foobar
+    assert_equal Mime[:html], Mime::EXTENSION_LOOKUP["foobar"]
+  ensure
+    Mime::Type.unregister(:foobar)
   end
 
   test "type should be equal to symbol" do
diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb
index beab8e78b5..2a48a12497 100644
--- a/actionpack/test/dispatch/request/json_params_parsing_test.rb
+++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb
@@ -74,17 +74,15 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest
 
   test "occurring a parse error if parsing unsuccessful" do
     with_test_routing do
-      begin
-        $stderr = StringIO.new # suppress the log
-        json = "[\"person]\": {\"name\": \"David\"}}"
-        exception = assert_raise(ActionDispatch::Http::Parameters::ParseError) do
-          post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false }
-        end
-        assert_equal JSON::ParserError, exception.cause.class
-        assert_equal exception.cause.message, exception.message
-      ensure
-        $stderr = STDERR
+      $stderr = StringIO.new # suppress the log
+      json = "[\"person]\": {\"name\": \"David\"}}"
+      exception = assert_raise(ActionDispatch::Http::Parameters::ParseError) do
+        post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => false }
       end
+      assert_equal JSON::ParserError, exception.cause.class
+      assert_equal exception.cause.message, exception.message
+    ensure
+      $stderr = STDERR
     end
   end
 
@@ -157,31 +155,27 @@ class RootLessJSONParamsParsingTest < ActionDispatch::IntegrationTest
   end
 
   test "parses json params after custom json mime type registered" do
-    begin
-      Mime::Type.unregister :json
-      Mime::Type.register "application/json", :json, %w(application/vnd.rails+json)
-      assert_parses(
-        { "user" => { "username" => "meinac" }, "username" => "meinac" },
-        "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/json"
-      )
-    ensure
-      Mime::Type.unregister :json
-      Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest )
-    end
+    Mime::Type.unregister :json
+    Mime::Type.register "application/json", :json, %w(application/vnd.rails+json)
+    assert_parses(
+      { "user" => { "username" => "meinac" }, "username" => "meinac" },
+      "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/json"
+    )
+  ensure
+    Mime::Type.unregister :json
+    Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest )
   end
 
   test "parses json params after custom json mime type registered with synonym" do
-    begin
-      Mime::Type.unregister :json
-      Mime::Type.register "application/json", :json, %w(application/vnd.rails+json)
-      assert_parses(
-        { "user" => { "username" => "meinac" }, "username" => "meinac" },
-        "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/vnd.rails+json"
-      )
-    ensure
-      Mime::Type.unregister :json
-      Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest )
-    end
+    Mime::Type.unregister :json
+    Mime::Type.register "application/json", :json, %w(application/vnd.rails+json)
+    assert_parses(
+      { "user" => { "username" => "meinac" }, "username" => "meinac" },
+      "{\"username\": \"meinac\"}", "CONTENT_TYPE" => "application/vnd.rails+json"
+    )
+  ensure
+    Mime::Type.unregister :json
+    Mime::Type.register "application/json", :json, %w( text/x-json application/jsonrequest )
   end
 
   private
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index c7b68e5266..9d1246b3a4 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -1059,47 +1059,9 @@ class RequestParameters < BaseRequestTest
 end
 
 class RequestParameterFilter < BaseRequestTest
-  test "process parameter filter" do
-    test_hashes = [
-    [{ "foo" => "bar" }, { "foo" => "bar" }, %w'food'],
-    [{ "foo" => "bar" }, { "foo" => "[FILTERED]" }, %w'foo'],
-    [{ "foo" => "bar", "bar" => "foo" }, { "foo" => "[FILTERED]", "bar" => "foo" }, %w'foo baz'],
-    [{ "foo" => "bar", "baz" => "foo" }, { "foo" => "[FILTERED]", "baz" => "[FILTERED]" }, %w'foo baz'],
-    [{ "bar" => { "foo" => "bar", "bar" => "foo" } }, { "bar" => { "foo" => "[FILTERED]", "bar" => "foo" } }, %w'fo'],
-    [{ "foo" => { "foo" => "bar", "bar" => "foo" } }, { "foo" => "[FILTERED]" }, %w'f banana'],
-    [{ "deep" => { "cc" => { "code" => "bar", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, { "deep" => { "cc" => { "code" => "[FILTERED]", "bar" => "foo" }, "ss" => { "code" => "bar" } } }, %w'deep.cc.code'],
-    [{ "baz" => [{ "foo" => "baz" }, "1"] }, { "baz" => [{ "foo" => "[FILTERED]" }, "1"] }, [/foo/]]]
-
-    test_hashes.each do |before_filter, after_filter, filter_words|
-      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
-      assert_equal after_filter, parameter_filter.filter(before_filter)
-
-      filter_words << "blah"
-      filter_words << lambda { |key, value|
-        value.reverse! if key =~ /bargain/
-      }
-      filter_words << lambda { |key, value, original_params|
-        value.replace("world!") if original_params["barg"]["blah"] == "bar" && key == "hello"
-      }
-
-      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
-      before_filter["barg"] = { :bargain => "gain", "blah" => "bar", "bar" => { "bargain" => { "blah" => "foo", "hello" => "world" } } }
-      after_filter["barg"]  = { :bargain => "niag", "blah" => "[FILTERED]", "bar" => { "bargain" => { "blah" => "[FILTERED]", "hello" => "world!" } } }
-
-      assert_equal after_filter, parameter_filter.filter(before_filter)
-    end
-  end
-
-  test "parameter filter should maintain hash with indifferent access" do
-    test_hashes = [
-      [{ "foo" => "bar" }.with_indifferent_access, ["blah"]],
-      [{ "foo" => "bar" }.with_indifferent_access, []]
-    ]
-
-    test_hashes.each do |before_filter, filter_words|
-      parameter_filter = ActionDispatch::Http::ParameterFilter.new(filter_words)
-      assert_instance_of ActiveSupport::HashWithIndifferentAccess,
-                         parameter_filter.filter(before_filter)
+  test "parameter filter is deprecated" do
+    assert_deprecated do
+      ActionDispatch::Http::ParameterFilter.new(["blah"])
     end
   end
 
diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb
index 0f37d074af..60817c1c4d 100644
--- a/actionpack/test/dispatch/response_test.rb
+++ b/actionpack/test/dispatch/response_test.rb
@@ -42,7 +42,7 @@ class ResponseTest < ActiveSupport::TestCase
   def test_each_isnt_called_if_str_body_is_written
     # Controller writes and reads response body
     each_counter = 0
-    @response.body = Object.new.tap { |o| o.singleton_class.send(:define_method, :each) { |&block| each_counter += 1; block.call "foo" } }
+    @response.body = Object.new.tap { |o| o.singleton_class.define_method(:each) { |&block| each_counter += 1; block.call "foo" } }
     @response["X-Foo"] = @response.body
 
     assert_equal 1, each_counter, "#each was not called once"
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 5efbe5b553..4dffbd0db1 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -115,6 +115,21 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal 301, status
   end
 
+  def test_accepts_a_constraint_object_responding_to_call
+    constraint = Class.new do
+      def call(*); true; end
+      def matches?(*); false; end
+    end
+
+    draw do
+      get "/", to: "home#show", constraints: constraint.new
+    end
+
+    assert_nothing_raised do
+      get "/"
+    end
+  end
+
   def test_namespace_with_controller_segment
     assert_raise(ArgumentError) do
       draw do
@@ -1367,6 +1382,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     assert_equal "projects#index", @response.body
   end
 
+  def test_optionally_scoped_root_unscoped_access
+    draw do
+      scope "(:locale)" do
+        scope "(:platform)" do
+          scope "(:browser)" do
+            root to: "projects#index"
+          end
+        end
+      end
+    end
+
+    assert_equal "/", root_path
+    get "/"
+    assert_equal "projects#index", @response.body
+  end
+
   def test_scope_with_format_option
     draw do
       get "direct/index", as: :no_format_direct, format: false
@@ -3667,15 +3698,25 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
-  def test_multiple_roots
+  def test_multiple_roots_raises_error
+    ex = assert_raises(ArgumentError) {
+      draw do
+        root "pages#index", constraints: { host: "www.example.com" }
+        root "admin/pages#index", constraints: { host: "admin.example.com" }
+      end
+    }
+    assert_match(/Invalid route name, already in use: 'root'/, ex.message)
+  end
+
+  def test_multiple_named_roots
     draw do
       namespace :foo do
         root "pages#index", constraints: { host: "www.example.com" }
-        root "admin/pages#index", constraints: { host: "admin.example.com" }
+        root "admin/pages#index", constraints: { host: "admin.example.com" }, as: :admin_root
       end
 
       root "pages#index", constraints: { host: "www.example.com" }
-      root "admin/pages#index", constraints: { host: "admin.example.com" }
+      root "admin/pages#index", constraints: { host: "admin.example.com" },  as: :admin_root
     end
 
     get "http://www.example.com/foo"
diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb
index b69071b44b..f802abc653 100644
--- a/actionpack/test/dispatch/show_exceptions_test.rb
+++ b/actionpack/test/dispatch/show_exceptions_test.rb
@@ -36,30 +36,30 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
   test "skip exceptions app if not showing exceptions" do
     @app = ProductionApp
     assert_raise RuntimeError do
-      get "/", headers: { "action_dispatch.show_exceptions" => false }
+      get "/", env: { "action_dispatch.show_exceptions" => false }
     end
   end
 
   test "rescue with error page" do
     @app = ProductionApp
 
-    get "/", headers: { "action_dispatch.show_exceptions" => true }
+    get "/", env: { "action_dispatch.show_exceptions" => true }
     assert_response 500
     assert_equal "500 error fixture\n", body
 
-    get "/bad_params", headers: { "action_dispatch.show_exceptions" => true }
+    get "/bad_params", env: { "action_dispatch.show_exceptions" => true }
     assert_response 400
     assert_equal "400 error fixture\n", body
 
-    get "/not_found", headers: { "action_dispatch.show_exceptions" => true }
+    get "/not_found", env: { "action_dispatch.show_exceptions" => true }
     assert_response 404
     assert_equal "404 error fixture\n", body
 
-    get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true }
+    get "/method_not_allowed", env: { "action_dispatch.show_exceptions" => true }
     assert_response 405
     assert_equal "", body
 
-    get "/unknown_http_method", headers: { "action_dispatch.show_exceptions" => true }
+    get "/unknown_http_method", env: { "action_dispatch.show_exceptions" => true }
     assert_response 405
     assert_equal "", body
   end
@@ -70,11 +70,11 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
     begin
       @app = ProductionApp
 
-      get "/", headers: { "action_dispatch.show_exceptions" => true }
+      get "/", env: { "action_dispatch.show_exceptions" => true }
       assert_response 500
       assert_equal "500 localized error fixture\n", body
 
-      get "/not_found", headers: { "action_dispatch.show_exceptions" => true }
+      get "/not_found", env: { "action_dispatch.show_exceptions" => true }
       assert_response 404
       assert_equal "404 error fixture\n", body
     ensure
@@ -85,14 +85,14 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
   test "sets the HTTP charset parameter" do
     @app = ProductionApp
 
-    get "/", headers: { "action_dispatch.show_exceptions" => true }
+    get "/", env: { "action_dispatch.show_exceptions" => true }
     assert_equal "text/html; charset=utf-8", response.headers["Content-Type"]
   end
 
   test "show registered original exception for wrapped exceptions" do
     @app = ProductionApp
 
-    get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true }
+    get "/not_found_original_exception", env: { "action_dispatch.show_exceptions" => true }
     assert_response 404
     assert_match(/404 error/, body)
   end
@@ -106,7 +106,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
     end
 
     @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app)
-    get "/not_found_original_exception", headers: { "action_dispatch.show_exceptions" => true }
+    get "/not_found_original_exception", env: { "action_dispatch.show_exceptions" => true }
     assert_response 404
     assert_equal "YOU FAILED", body
   end
@@ -117,7 +117,7 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
     end
 
     @app = ActionDispatch::ShowExceptions.new(Boomer.new, exceptions_app)
-    get "/method_not_allowed", headers: { "action_dispatch.show_exceptions" => true }
+    get "/method_not_allowed", env: { "action_dispatch.show_exceptions" => true }
     assert_response 405
     assert_equal "", body
   end
@@ -125,12 +125,12 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest
   test "bad params exception is returned in the correct format" do
     @app = ProductionApp
 
-    get "/bad_params", headers: { "action_dispatch.show_exceptions" => true }
+    get "/bad_params", env: { "action_dispatch.show_exceptions" => true }
     assert_equal "text/html; charset=utf-8", response.headers["Content-Type"]
     assert_response 400
     assert_match(/400 error/, body)
 
-    get "/bad_params.json", headers: { "action_dispatch.show_exceptions" => true }
+    get "/bad_params.json", env: { "action_dispatch.show_exceptions" => true }
     assert_equal "application/json; charset=utf-8", response.headers["Content-Type"]
     assert_response 400
     assert_equal("{\"status\":400,\"error\":\"Bad Request\"}", body)
diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
index de79c05657..097ef8af29 100644
--- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
+++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb
@@ -41,22 +41,20 @@ class ScreenshotHelperTest < ActiveSupport::TestCase
   end
 
   test "display_image return artifact format when specify RAILS_SYSTEM_TESTING_SCREENSHOT environment" do
-    begin
-      original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"]
-      ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact"
+    original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"]
+    ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact"
 
-      new_test = DrivenBySeleniumWithChrome.new("x")
+    new_test = DrivenBySeleniumWithChrome.new("x")
 
-      assert_equal "artifact", new_test.send(:output_type)
+    assert_equal "artifact", new_test.send(:output_type)
 
-      Rails.stub :root, Pathname.getwd do
-        new_test.stub :passed?, false do
-          assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image)
-        end
+    Rails.stub :root, Pathname.getwd do
+      new_test.stub :passed?, false do
+        assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image)
       end
-    ensure
-      ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = original_output_type
     end
+  ensure
+    ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = original_output_type
   end
 
   test "image path returns the absolute path from root" do