Merge pull request #430 from dlee/methodoverride

Logs should show overridden method; Issue 426

actionpack/test/dispatch/request_test.rb

@@ -358,6 +358,13 @@ class RequestTest < ActiveSupport::TestCase
     assert request.head?
   end
 
+  test "post masquerading as put" do
+    request = stub_request 'REQUEST_METHOD' => 'PUT', "rack.methodoverride.original_method" => "POST"
+    assert_equal "POST", request.method
+    assert_equal "PUT",  request.request_method
+    assert request.put?
+  end
+
   test "xml format" do
     request = stub_request
     request.expects(:parameters).at_least_once.returns({ :format => 'xml' })

activesupport/lib/active_support/log_subscriber/test_helper.rb

@@ -1,5 +1,6 @@
 require 'active_support/log_subscriber'
 require 'active_support/buffered_logger'
+require 'active_support/notifications'
 
 module ActiveSupport
   class LogSubscriber

railties/lib/rails/application.rb

@@ -157,7 +157,8 @@ module Rails
 
         middleware.use ::Rack::Lock unless config.allow_concurrency
         middleware.use ::Rack::Runtime
-        middleware.use ::Rails::Rack::Logger
+        middleware.use ::Rack::MethodOverride
+        middleware.use ::Rails::Rack::Logger # must come after Rack::MethodOverride to properly log overridden methods
         middleware.use ::ActionDispatch::ShowExceptions, config.consider_all_requests_local
         middleware.use ::ActionDispatch::RemoteIp, config.action_dispatch.ip_spoofing_check, config.action_dispatch.trusted_proxies
         middleware.use ::Rack::Sendfile, config.action_dispatch.x_sendfile_header
@@ -171,7 +172,6 @@ module Rails
         end
 
         middleware.use ::ActionDispatch::ParamsParser
-        middleware.use ::Rack::MethodOverride
         middleware.use ::ActionDispatch::Head
         middleware.use ::Rack::ConditionalGet
         middleware.use ::Rack::ETag, "no-cache"
@@ -199,4 +199,4 @@ module Rails
       require "rails/console/helpers"
     end
   end
-end
+end

railties/test/application/middleware_test.rb

@@ -23,7 +23,8 @@ module ApplicationTests
         "Rack::Lock",
         "ActiveSupport::Cache::Strategy::LocalCache",
         "Rack::Runtime",
-        "Rails::Rack::Logger",
+        "Rack::MethodOverride",
+        "Rails::Rack::Logger", # must come after Rack::MethodOverride to properly log overridden methods
         "ActionDispatch::ShowExceptions",
         "ActionDispatch::RemoteIp",
         "Rack::Sendfile",
@@ -36,7 +37,6 @@ module ApplicationTests
         "ActionDispatch::Session::CookieStore",
         "ActionDispatch::Flash",
         "ActionDispatch::ParamsParser",
-        "Rack::MethodOverride",
         "ActionDispatch::Head",
         "Rack::ConditionalGet",
         "Rack::ETag",

railties/test/application/rack/logger_test.rb

@@ -0,0 +1,40 @@
+require "isolation/abstract_unit"
+require "active_support/log_subscriber/test_helper"
+require "rack/test"
+
+module ApplicationTests
+  module RackTests
+    class LoggerTest < Test::Unit::TestCase
+      include ActiveSupport::LogSubscriber::TestHelper
+      include Rack::Test::Methods
+
+      def setup
+        build_app
+        require "#{app_path}/config/environment"
+        super
+      end
+
+      def logs
+        @logs ||= @logger.logged(:info)
+      end
+
+      test "logger logs proper HTTP verb and path" do
+        get "/blah"
+        wait
+        assert_match /^Started GET "\/blah"/, logs[0]
+      end
+
+      test "logger logs HTTP verb override" do
+        post "/", {:_method => 'put'}
+        wait
+        assert_match /^Started PUT "\/"/, logs[0]
+      end
+
+      test "logger logs HEAD requests" do
+        post "/", {:_method => 'head'}
+        wait
+        assert_match /^Started HEAD "\/"/, logs[0]
+      end
+    end
+  end
+end