Merge branch 'master' into bug/filtered_parameters_class

10 files changed, 253 insertions, 19 deletions

diff --git a/actionpack/test/controller/api/with_helpers_test.rb b/actionpack/test/controller/api/with_helpers_test.rb
new file mode 100644
index 0000000000..06db949153
--- /dev/null
+++ b/actionpack/test/controller/api/with_helpers_test.rb
@@ -0,0 +1,42 @@
+require "abstract_unit"
+
+module ApiWithHelper
+  def my_helper
+    "helper"
+  end
+end
+
+class WithHelpersController < ActionController::API
+  include ActionController::Helpers
+  helper ApiWithHelper
+
+  def with_helpers
+    render plain: self.class.helpers.my_helper
+  end
+end
+
+class SubclassWithHelpersController < WithHelpersController
+  def with_helpers
+    render plain: self.class.helpers.my_helper
+  end
+end
+
+class WithHelpersTest < ActionController::TestCase
+  tests WithHelpersController
+
+  def test_with_helpers
+    get :with_helpers
+
+    assert_equal "helper", response.body
+  end
+end
+
+class SubclassWithHelpersTest < ActionController::TestCase
+  tests WithHelpersController
+
+  def test_with_helpers
+    get :with_helpers
+
+    assert_equal "helper", response.body
+  end
+end
diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb
index 42a5157010..4e969fac07 100644
--- a/actionpack/test/controller/base_test.rb
+++ b/actionpack/test/controller/base_test.rb
@@ -11,6 +11,12 @@ end
 class EmptyController < ActionController::Base
 end
 
+class SimpleController < ActionController::Base
+  def hello
+    self.response_body = "hello"
+  end
+end
+
 class NonEmptyController < ActionController::Base
   def public_action
     head :ok
@@ -118,6 +124,27 @@ class ControllerInstanceTests < ActiveSupport::TestCase
     controller = klass.new
     assert_equal "examples", controller.controller_path
   end
+
+  def test_response_has_default_headers
+    original_default_headers = ActionDispatch::Response.default_headers
+
+    ActionDispatch::Response.default_headers = {
+      "X-Frame-Options" => "DENY",
+      "X-Content-Type-Options" => "nosniff",
+      "X-XSS-Protection" => "1;"
+    }
+
+    response_headers = SimpleController.action("hello").call(
+      "REQUEST_METHOD" => "GET",
+      "rack.input" => -> {}
+    )[1]
+
+    assert response_headers.key?("X-Frame-Options")
+    assert response_headers.key?("X-Content-Type-Options")
+    assert response_headers.key?("X-XSS-Protection")
+  ensure
+    ActionDispatch::Response.default_headers = original_default_headers
+  end
 end
 
 class PerformActionTest < ActionController::TestCase
diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb
index e76628b936..581081dd07 100644
--- a/actionpack/test/controller/live_stream_test.rb
+++ b/actionpack/test/controller/live_stream_test.rb
@@ -1,4 +1,5 @@
 require "abstract_unit"
+require "timeout"
 require "concurrent/atomic/count_down_latch"
 Thread.abort_on_exception = true
 
diff --git a/actionpack/test/controller/metal_test.rb b/actionpack/test/controller/metal_test.rb
new file mode 100644
index 0000000000..e16452ed6f
--- /dev/null
+++ b/actionpack/test/controller/metal_test.rb
@@ -0,0 +1,30 @@
+require "abstract_unit"
+
+class MetalControllerInstanceTests < ActiveSupport::TestCase
+  class SimpleController < ActionController::Metal
+    def hello
+      self.response_body = "hello"
+    end
+  end
+
+  def test_response_has_default_headers
+    original_default_headers = ActionDispatch::Response.default_headers
+
+    ActionDispatch::Response.default_headers = {
+      "X-Frame-Options" => "DENY",
+      "X-Content-Type-Options" => "nosniff",
+      "X-XSS-Protection" => "1;"
+    }
+
+    response_headers = SimpleController.action("hello").call(
+      "REQUEST_METHOD" => "GET",
+      "rack.input" => -> {}
+    )[1]
+
+    refute response_headers.key?("X-Frame-Options")
+    refute response_headers.key?("X-Content-Type-Options")
+    refute response_headers.key?("X-XSS-Protection")
+  ensure
+    ActionDispatch::Response.default_headers = original_default_headers
+  end
+end
diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb
index 2893eb7b91..7725c25e22 100644
--- a/actionpack/test/controller/parameters/accessors_test.rb
+++ b/actionpack/test/controller/parameters/accessors_test.rb
@@ -53,6 +53,15 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
     @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" }
   end
 
+  test "empty? returns true when params contains no key/value pairs" do
+    params = ActionController::Parameters.new
+    assert params.empty?
+  end
+
+  test "empty? returns false when any params are present" do
+    refute @params.empty?
+  end
+
   test "except retains permitted status" do
     @params.permit!
     assert @params.except(:person).permitted?
@@ -75,6 +84,45 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
     assert_not @params[:person].fetch(:name).permitted?
   end
 
+  test "has_key? returns true if the given key is present in the params" do
+    assert @params.has_key?(:person)
+  end
+
+  test "has_key? returns false if the given key is not present in the params" do
+    refute @params.has_key?(:address)
+  end
+
+  test "has_value? returns true if the given value is present in the params" do
+    params = ActionController::Parameters.new(city: "Chicago", state: "Illinois")
+    assert params.has_value?("Chicago")
+  end
+
+  test "has_value? returns false if the given value is not present in the params" do
+    params = ActionController::Parameters.new(city: "Chicago", state: "Illinois")
+    refute params.has_value?("New York")
+  end
+
+  test "include? returns true if the given key is present in the params" do
+    assert @params.include?(:person)
+  end
+
+  test "include? returns false if the given key is not present in the params" do
+    refute @params.include?(:address)
+  end
+
+  test "key? returns true if the given key is present in the params" do
+    assert @params.key?(:person)
+  end
+
+  test "key? returns false if the given key is not present in the params" do
+    refute @params.key?(:address)
+  end
+
+  test "keys returns an array of the keys of the params" do
+    assert_equal ["person"], @params.keys
+    assert_equal ["age", "name", "addresses"], @params[:person].keys
+  end
+
   test "reject retains permitted status" do
     assert_not @params.reject { |k| k == "person" }.permitted?
   end
@@ -120,6 +168,21 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
     assert_not @params.transform_values { |v| v }.permitted?
   end
 
+  test "value? returns true if the given value is present in the params" do
+    params = ActionController::Parameters.new(city: "Chicago", state: "Illinois")
+    assert params.value?("Chicago")
+  end
+
+  test "value? returns false if the given value is not present in the params" do
+    params = ActionController::Parameters.new(city: "Chicago", state: "Illinois")
+    refute params.value?("New York")
+  end
+
+  test "values returns an array of the values of the params" do
+    params = ActionController::Parameters.new(city: "Chicago", state: "Illinois")
+    assert_equal ["Chicago", "Illinois"], params.values
+  end
+
   test "values_at retains permitted status" do
     @params.permit!
     assert @params.values_at(:person).first.permitted?
diff --git a/actionpack/test/controller/parameters/parameters_permit_test.rb b/actionpack/test/controller/parameters/parameters_permit_test.rb
index 8920914af1..3e067314d6 100644
--- a/actionpack/test/controller/parameters/parameters_permit_test.rb
+++ b/actionpack/test/controller/parameters/parameters_permit_test.rb
@@ -302,6 +302,47 @@ class ParametersPermitTest < ActiveSupport::TestCase
     assert_equal "32", @params[:person][:age]
   end
 
+  test "#reverse_merge with parameters" do
+    default_params = ActionController::Parameters.new(id: "1234", person: {}).permit!
+    merged_params = @params.reverse_merge(default_params)
+
+    assert_equal "1234", merged_params[:id]
+    refute_predicate merged_params[:person], :empty?
+  end
+
+  test "#with_defaults is an alias of reverse_merge" do
+    default_params = ActionController::Parameters.new(id: "1234", person: {}).permit!
+    merged_params = @params.with_defaults(default_params)
+
+    assert_equal "1234", merged_params[:id]
+    refute_predicate merged_params[:person], :empty?
+  end
+
+  test "not permitted is sticky beyond reverse_merge" do
+    refute_predicate @params.reverse_merge(a: "b"), :permitted?
+  end
+
+  test "permitted is sticky beyond reverse_merge" do
+    @params.permit!
+    assert_predicate @params.reverse_merge(a: "b"), :permitted?
+  end
+
+  test "#reverse_merge! with parameters" do
+    default_params = ActionController::Parameters.new(id: "1234", person: {}).permit!
+    @params.reverse_merge!(default_params)
+
+    assert_equal "1234", @params[:id]
+    refute_predicate @params[:person], :empty?
+  end
+
+  test "#with_defaults! is an alias of reverse_merge!" do
+    default_params = ActionController::Parameters.new(id: "1234", person: {}).permit!
+    @params.with_defaults!(default_params)
+
+    assert_equal "1234", @params[:id]
+    refute_predicate @params[:person], :empty?
+  end
+
   test "modifying the parameters" do
     @params[:person][:hometown] = "Chicago"
     @params[:person][:family] = { brother: "Jonas" }
diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb
index faa57c4559..2a41d57b26 100644
--- a/actionpack/test/controller/params_wrapper_test.rb
+++ b/actionpack/test/controller/params_wrapper_test.rb
@@ -32,6 +32,10 @@ class ParamsWrapperTest < ActionController::TestCase
     def self.attribute_names
       []
     end
+
+    def self.stored_attributes
+      { settings: [:color, :size] }
+    end
   end
 
   class Person
@@ -62,6 +66,17 @@ class ParamsWrapperTest < ActionController::TestCase
     end
   end
 
+  def test_store_accessors_wrapped
+    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", "color" => "blue", "size" => "large" }
+        assert_parameters("username" => "sikachu", "color" => "blue", "size" => "large",
+                           "user" => { "username" => "sikachu", "color" => "blue", "size" => "large" })
+      end
+    end
+  end
+
   def test_specify_wrapper_name
     with_default_wrapper_options do
       UsersController.wrap_parameters :person
diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb
index e4e968dfdb..f06a1f4d23 100644
--- a/actionpack/test/controller/redirect_test.rb
+++ b/actionpack/test/controller/redirect_test.rb
@@ -21,8 +21,8 @@ end
 class RedirectController < ActionController::Base
   # empty method not used anywhere to ensure methods like
   # `status` and `location` aren't called on `redirect_to` calls
-  def status; render plain: "called status"; end
-  def location; render plain: "called location"; end
+  def status; raise "Should not be called!"; end
+  def location; raise "Should not be called!"; end
 
   def simple_redirect
     redirect_to action: "hello_world"
diff --git a/actionpack/test/controller/renderer_test.rb b/actionpack/test/controller/renderer_test.rb
index 866600b935..052c974d68 100644
--- a/actionpack/test/controller/renderer_test.rb
+++ b/actionpack/test/controller/renderer_test.rb
@@ -19,6 +19,16 @@ class RendererTest < ActiveSupport::TestCase
     assert_equal controller, renderer.controller
   end
 
+  test "creating with new defaults" do
+    renderer = ApplicationController.renderer
+
+    new_defaults = { https: true }
+    new_renderer = renderer.with_defaults(new_defaults).new
+    content = new_renderer.render(inline: "<%= request.ssl? %>")
+
+    assert_equal "true", content
+  end
+
   test "rendering with a class renderer" do
     renderer = ApplicationController.renderer
     content  = renderer.render template: "ruby_template"
@@ -103,6 +113,20 @@ class RendererTest < ActiveSupport::TestCase
     assert_equal "true", content
   end
 
+  test "return valid asset url with defaults" do
+    renderer = ApplicationController.renderer
+    content  = renderer.render inline: "<%= asset_url 'asset.jpg' %>"
+
+    assert_equal "http://example.org/asset.jpg", content
+  end
+
+  test "return valid asset url when https is true" do
+    renderer = ApplicationController.renderer.new https: true
+    content  = renderer.render inline: "<%= asset_url 'asset.jpg' %>"
+
+    assert_equal "https://example.org/asset.jpg", content
+  end
+
   private
     def render
       @render ||= ApplicationController.renderer.method(:render)
diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb
index e084e45373..3a4307b64b 100644
--- a/actionpack/test/controller/test_case_test.rb
+++ b/actionpack/test/controller/test_case_test.rb
@@ -134,7 +134,7 @@ XML
     end
 
     def create
-      head :created, location: "created resource"
+      head :created, location: "/resource"
     end
 
     def render_cookie
@@ -679,6 +679,11 @@ XML
     assert_equal "baz", @request.filtered_parameters[:foo]
   end
 
+  def test_path_is_kept_after_the_request
+    get :test_params, params: { id: "foo" }
+    assert_equal "/test_case_test/test/test_params/foo", @request.path
+  end
+
   def test_path_params_reset_between_request
     get :test_params, params: { id: "foo" }
     assert_equal "foo", @request.path_parameters[:id]
@@ -728,20 +733,6 @@ XML
     assert_equal "text/html", @response.body
   end
 
-  def test_request_path_info_and_format_reset
-    get :test_format, format: "json"
-    assert_equal "application/json", @response.body
-
-    get :test_uri, format: "json"
-    assert_equal "/test_case_test/test/test_uri.json", @response.body
-
-    get :test_format
-    assert_equal "text/html", @response.body
-
-    get :test_uri
-    assert_equal "/test_case_test/test/test_uri", @response.body
-  end
-
   def test_request_format_kwarg_overrides_params
     get :test_format, format: "json", params: { format: "html" }
     assert_equal "application/json", @response.body
@@ -893,12 +884,12 @@ XML
     assert_response :created
 
     # Redirect url doesn't care that it wasn't a :redirect response.
-    assert_equal "created resource", @response.redirect_url
+    assert_equal "/resource", @response.redirect_url
     assert_equal @response.redirect_url, redirect_to_url
 
     # Must be a :redirect response.
     assert_raise(ActiveSupport::TestCase::Assertion) do
-      assert_redirected_to "created resource"
+      assert_redirected_to "/resource"
     end
   end