Merge branch 'master' into require_relative_2017

11 files changed, 91 insertions, 31 deletions

diff --git a/actionpack/lib/action_controller/log_subscriber.rb b/actionpack/lib/action_controller/log_subscriber.rb
index d00fcbcd13..5d75393897 100644
--- a/actionpack/lib/action_controller/log_subscriber.rb
+++ b/actionpack/lib/action_controller/log_subscriber.rb
@@ -24,7 +24,7 @@ module ActionController
           exception_class_name = payload[:exception].first
           status = ActionDispatch::ExceptionWrapper.status_code_for_exception(exception_class_name)
         end
-        message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms"
+        message = "Completed #{status} #{Rack::Utils::HTTP_STATUS_CODES[status]} in #{event.duration.round}ms".dup
         message << " (#{additions.join(" | ".freeze)})" unless additions.empty?
         message << "\n\n" if defined?(Rails.env) && Rails.env.development?
 
diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb
index 44151c9f71..818af549eb 100644
--- a/actionpack/lib/action_controller/metal/params_wrapper.rb
+++ b/actionpack/lib/action_controller/metal/params_wrapper.rb
@@ -232,12 +232,7 @@ module ActionController
     # by the metal call stack.
     def process_action(*args)
       if _wrapper_enabled?
-        if request.parameters[_wrapper_key].present?
-          wrapped_hash = _extract_parameters(request.parameters)
-        else
-          wrapped_hash = _wrap_parameters request.request_parameters
-        end
-
+        wrapped_hash = _wrap_parameters request.request_parameters
         wrapped_keys = request.request_parameters.keys
         wrapped_filtered_hash = _wrap_parameters request.filtered_parameters.slice(*wrapped_keys)
 
@@ -282,7 +277,7 @@ module ActionController
         return false unless request.has_content_type?
 
         ref = request.content_mime_type.ref
-        _wrapper_formats.include?(ref) && _wrapper_key && !request.request_parameters.key?(_wrapper_key)
+        _wrapper_formats.include?(ref) && _wrapper_key && !request.parameters.key?(_wrapper_key)
       end
   end
 end
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 0007744298..eab663e2e0 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -103,7 +103,7 @@ module ActionDispatch # :nodoc:
 
       def body
         @str_body ||= begin
-          buf = ""
+          buf = "".dup
           each { |chunk| buf << chunk }
           buf
         end
diff --git a/actionpack/lib/action_dispatch/journey/router/utils.rb b/actionpack/lib/action_dispatch/journey/router/utils.rb
index 6d400f3364..1ac86d10d6 100644
--- a/actionpack/lib/action_dispatch/journey/router/utils.rb
+++ b/actionpack/lib/action_dispatch/journey/router/utils.rb
@@ -14,11 +14,11 @@ module ActionDispatch
         #   normalize_path("/%ab")  # => "/%AB"
         def self.normalize_path(path)
           encoding = path.encoding
-          path = "/#{path}"
+          path = "/#{path}".dup
           path.squeeze!("/".freeze)
           path.sub!(%r{/+\Z}, "".freeze)
           path.gsub!(/(%[a-f0-9]{2})/) { $1.upcase }
-          path = "/" if path == "".freeze
+          path = "/".dup if path == "".freeze
           path.force_encoding(encoding)
           path
         end
@@ -29,7 +29,7 @@ module ActionDispatch
           ENCODE   = "%%%02X".freeze
           US_ASCII = Encoding::US_ASCII
           UTF_8    = Encoding::UTF_8
-          EMPTY    = "".force_encoding(US_ASCII).freeze
+          EMPTY    = "".dup.force_encoding(US_ASCII).freeze
           DEC2HEX  = (0..255).to_a.map { |i| ENCODE % i }.map { |s| s.force_encoding(US_ASCII) }
 
           ALPHA = "a-zA-Z".freeze
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 1be2bc8022..ebe809f64e 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -73,7 +73,6 @@ module ActionDispatch
           @routes = {}
           @path_helpers = Set.new
           @url_helpers = Set.new
-          @custom_helpers = Set.new
           @url_helpers_module  = Module.new
           @path_helpers_module = Module.new
         end
@@ -96,23 +95,9 @@ module ActionDispatch
             @url_helpers_module.send  :remove_method, helper
           end
 
-          @custom_helpers.each do |helper|
-            path_name = :"#{helper}_path"
-            url_name = :"#{helper}_url"
-
-            if @path_helpers_module.method_defined?(path_name)
-              @path_helpers_module.send :remove_method, path_name
-            end
-
-            if @url_helpers_module.method_defined?(url_name)
-              @url_helpers_module.send :remove_method, url_name
-            end
-          end
-
           @routes.clear
           @path_helpers.clear
           @url_helpers.clear
-          @custom_helpers.clear
         end
 
         def add(name, route)
@@ -158,21 +143,29 @@ module ActionDispatch
           routes.length
         end
 
+        # Given a +name+, defines name_path and name_url helpers.
+        # Used by 'direct', 'resolve', and 'polymorphic' route helpers.
         def add_url_helper(name, defaults, &block)
-          @custom_helpers << name
           helper = CustomUrlHelper.new(name, defaults, &block)
+          path_name = :"#{name}_path"
+          url_name = :"#{name}_url"
 
           @path_helpers_module.module_eval do
-            define_method(:"#{name}_path") do |*args|
+            define_method(path_name) do |*args|
               helper.call(self, args, true)
             end
           end
 
           @url_helpers_module.module_eval do
-            define_method(:"#{name}_url") do |*args|
+            define_method(url_name) do |*args|
               helper.call(self, args, false)
             end
           end
+
+          @path_helpers << path_name
+          @url_helpers << url_name
+
+          self
         end
 
         class UrlHelper
diff --git a/actionpack/lib/action_dispatch/system_test_case.rb b/actionpack/lib/action_dispatch/system_test_case.rb
index bc2d97d9d6..23ce8d5a92 100644
--- a/actionpack/lib/action_dispatch/system_test_case.rb
+++ b/actionpack/lib/action_dispatch/system_test_case.rb
@@ -5,6 +5,7 @@ require_relative "system_testing/driver"
 require_relative "system_testing/server"
 require_relative "system_testing/test_helpers/screenshot_helper"
 require_relative "system_testing/test_helpers/setup_and_teardown"
+require_relative "system_testing/test_helpers/undef_methods"
 
 module ActionDispatch
   # = System Testing
@@ -88,6 +89,7 @@ module ActionDispatch
     include Capybara::Minitest::Assertions
     include SystemTesting::TestHelpers::SetupAndTeardown
     include SystemTesting::TestHelpers::ScreenshotHelper
+    include SystemTesting::TestHelpers::UndefMethods
 
     def initialize(*) # :nodoc:
       super
diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/undef_methods.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/undef_methods.rb
new file mode 100644
index 0000000000..2d3f4662d7
--- /dev/null
+++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/undef_methods.rb
@@ -0,0 +1,24 @@
+module ActionDispatch
+  module SystemTesting
+    module TestHelpers
+      module UndefMethods # :nodoc:
+        extend ActiveSupport::Concern
+        included do
+          METHODS = %i(get post put patch delete).freeze
+
+          METHODS.each do |verb|
+            undef_method verb
+          end
+
+          def method_missing(method, *args, &block)
+            if METHODS.include?(method)
+              raise NoMethodError
+            else
+              super
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb
index 1baf979ac9..749f2eab57 100644
--- a/actionpack/lib/action_dispatch/testing/assertions/response.rb
+++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb
@@ -79,7 +79,7 @@ module ActionDispatch
         def generate_response_message(expected, actual = @response.response_code)
           "Expected response to be a <#{code_with_name(expected)}>,"\
           " but was a <#{code_with_name(actual)}>"
-            .concat(location_if_redirected).concat(response_body_if_short)
+            .dup.concat(location_if_redirected).concat(response_body_if_short)
         end
 
         def response_body_if_short
diff --git a/actionpack/test/controller/params_wrapper_test.rb b/actionpack/test/controller/params_wrapper_test.rb
index 4cbb28ef60..c0f01e6df8 100644
--- a/actionpack/test/controller/params_wrapper_test.rb
+++ b/actionpack/test/controller/params_wrapper_test.rb
@@ -226,6 +226,14 @@ class ParamsWrapperTest < ActionController::TestCase
     end
   end
 
+  def test_preserves_query_string_params_in_filtered_params
+    with_default_wrapper_options do
+      @request.env["CONTENT_TYPE"] = "application/json"
+      get :parse, params: { "user" => { "username" => "nixon" } }
+      assert_equal({ "controller" => "params_wrapper_test/users", "action" => "parse", "user" => { "username" => "nixon" } }, @request.filtered_parameters)
+    end
+  end
+
   def test_empty_parameter_set
     with_default_wrapper_options do
       @request.env["CONTENT_TYPE"] = "application/json"
diff --git a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
index 338992dda5..cbbed66056 100644
--- a/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
+++ b/actionpack/test/dispatch/routing/custom_url_helpers_test.rb
@@ -322,4 +322,10 @@ class TestCustomUrlHelpers < ActionDispatch::IntegrationTest
       end
     end
   end
+
+  def test_defining_direct_url_registers_helper_method
+    assert_equal "http://www.example.com/basket", Routes.url_helpers.symbol_url
+    assert_equal true, Routes.named_routes.route_defined?(:symbol_url), "'symbol_url' named helper not found"
+    assert_equal true, Routes.named_routes.route_defined?(:symbol_path), "'symbol_path' named helper not found"
+  end
 end
diff --git a/actionpack/test/dispatch/system_testing/system_test_case_test.rb b/actionpack/test/dispatch/system_testing/system_test_case_test.rb
index 8f90e45f5f..53f1a1bb37 100644
--- a/actionpack/test/dispatch/system_testing/system_test_case_test.rb
+++ b/actionpack/test/dispatch/system_testing/system_test_case_test.rb
@@ -31,3 +31,35 @@ class SetHostTest < DrivenByRackTest
     assert_equal "http://example.com", Capybara.app_host
   end
 end
+
+class UndefMethodsTest < DrivenBySeleniumWithChrome
+  test "get" do
+    assert_raise NoMethodError do
+      get "http://example.com"
+    end
+  end
+
+  test "post" do
+    assert_raise NoMethodError do
+      post "http://example.com"
+    end
+  end
+
+  test "put" do
+    assert_raise NoMethodError do
+      put "http://example.com"
+    end
+  end
+
+  test "patch" do
+    assert_raise NoMethodError do
+      patch "http://example.com"
+    end
+  end
+
+  test "delete" do
+    assert_raise NoMethodError do
+      delete "http://example.com"
+    end
+  end
+end