From bf9913f8f43a2436c644ad893d3d7034f7d7e256 Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Tue, 2 Mar 2010 16:22:53 -0800
Subject: [PATCH 01/19] Move session_store and session_options to the AC
 configuration object

---
 actionpack/lib/action_controller/metal.rb     | 17 +++++++++
 .../action_controller/metal/compatibility.rb  |  3 --
 .../metal/http_authentication.rb              | 28 +++++++--------
 .../metal/session_management.rb               | 35 +++++++++++++------
 .../http_digest_authentication_test.rb        |  5 +--
 5 files changed, 57 insertions(+), 31 deletions(-)

diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb
index 5e0ed201cb..fbc8a9a15e 100644
--- a/actionpack/lib/action_controller/metal.rb
+++ b/actionpack/lib/action_controller/metal.rb
@@ -1,4 +1,13 @@
 require 'active_support/core_ext/class/attribute'
+require 'active_support/ordered_options'
+
+module ActiveSupport
+  class InheritableOptions < OrderedOptions
+    def initialize(parent)
+      super() { |h,k| parent[k] }
+    end
+  end
+end
 
 module ActionController
   # ActionController::Metal provides a way to get a valid Rack application from a controller.
@@ -10,6 +19,14 @@ module ActionController
   class Metal < AbstractController::Base
     abstract!
 
+    def self.config
+      @config ||= ActiveSupport::InheritableOptions.new(superclass < Metal ? superclass.config : {})
+    end
+
+    def config
+      self.class.config
+    end
+
     # :api: public
     attr_internal :params, :env
 
diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 2b1ada1426..d1c86b296d 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -12,9 +12,6 @@ module ActionController
       ::ActionController::UnknownAction = ::AbstractController::ActionNotFound
       ::ActionController::DoubleRenderError = ::AbstractController::DoubleRenderError
 
-      cattr_accessor :session_options
-      self.session_options = {}
-
       cattr_accessor :relative_url_root
       self.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
 
diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb
index 0f35a7c040..5c6641c948 100644
--- a/actionpack/lib/action_controller/metal/http_authentication.rb
+++ b/actionpack/lib/action_controller/metal/http_authentication.rb
@@ -165,7 +165,7 @@ module ActionController
 
         # Authenticate with HTTP Digest, returns true or false
         def authenticate_with_http_digest(realm = "Application", &password_procedure)
-          HttpAuthentication::Digest.authenticate(request, realm, &password_procedure)
+          HttpAuthentication::Digest.authenticate(config.session_options[:secret], request, realm, &password_procedure)
         end
 
         # Render output including the HTTP Digest authentication header
@@ -175,8 +175,8 @@ module ActionController
       end
 
       # Returns false on a valid response, true otherwise
-      def authenticate(request, realm, &password_procedure)
-        authorization(request) && validate_digest_response(request, realm, &password_procedure)
+      def authenticate(secret_key, request, realm, &password_procedure)
+        authorization(request) && validate_digest_response(secret_key, request, realm, &password_procedure)
       end
 
       def authorization(request)
@@ -189,11 +189,11 @@ module ActionController
       # Returns false unless the request credentials response value matches the expected value.
       # First try the password as a ha1 digest password. If this fails, then try it as a plain
       # text password.
-      def validate_digest_response(request, realm, &password_procedure)
+      def validate_digest_response(secret_key, request, realm, &password_procedure)
         credentials = decode_credentials_header(request)
-        valid_nonce = validate_nonce(request, credentials[:nonce])
+        valid_nonce = validate_nonce(secret_key, request, credentials[:nonce])
 
-        if valid_nonce && realm == credentials[:realm] && opaque == credentials[:opaque]
+        if valid_nonce && realm == credentials[:realm] && opaque(secret_key) == credentials[:opaque]
           password = password_procedure.call(credentials[:username])
           return false unless password
 
@@ -238,6 +238,9 @@ module ActionController
       end
 
       def authentication_header(controller, realm)
+        secret_key = controller.config.session_options[:secret]
+        nonce = self.nonce(secret_key)
+        opaque = opaque(secret_key)
         controller.headers["WWW-Authenticate"] = %(Digest realm="#{realm}", qop="auth", algorithm=MD5, nonce="#{nonce}", opaque="#{opaque}")
       end
 
@@ -280,7 +283,7 @@ module ActionController
       # The nonce is opaque to the client. Composed of Time, and hash of Time with secret
       # key from the Rails session secret generated upon creation of project. Ensures
       # the time cannot be modified by client.
-      def nonce(time = Time.now)
+      def nonce(secret_key, time = Time.now)
         t = time.to_i
         hashed = [t, secret_key]
         digest = ::Digest::MD5.hexdigest(hashed.join(":"))
@@ -292,21 +295,16 @@ module ActionController
       # Can be much shorter if the Stale directive is implemented. This would
       # allow a user to use new nonce without prompting user again for their
       # username and password.
-      def validate_nonce(request, value, seconds_to_timeout=5*60)
+      def validate_nonce(secret_key, request, value, seconds_to_timeout=5*60)
         t = ActiveSupport::Base64.decode64(value).split(":").first.to_i
-        nonce(t) == value && (t - Time.now.to_i).abs <= seconds_to_timeout
+        nonce(secret_key, t) == value && (t - Time.now.to_i).abs <= seconds_to_timeout
       end
 
       # Opaque based on random generation - but changing each request?
-      def opaque()
+      def opaque(secret_key)
         ::Digest::MD5.hexdigest(secret_key)
       end
 
-      # Set in /initializers/session_store.rb, and loaded even if sessions are not in use.
-      def secret_key
-        ActionController::Base.session_options[:secret]
-      end
-
     end
   end
 end
diff --git a/actionpack/lib/action_controller/metal/session_management.rb b/actionpack/lib/action_controller/metal/session_management.rb
index 6997257844..264250db1a 100644
--- a/actionpack/lib/action_controller/metal/session_management.rb
+++ b/actionpack/lib/action_controller/metal/session_management.rb
@@ -2,27 +2,40 @@ module ActionController #:nodoc:
   module SessionManagement #:nodoc:
     extend ActiveSupport::Concern
 
+    included do
+      self.config.session_store   ||= :cookie_store
+      self.config.session_options ||= {}
+    end
+
     module ClassMethods
       # Set the session store to be used for keeping the session data between requests.
       # By default, sessions are stored in browser cookies (<tt>:cookie_store</tt>),
       # but you can also specify one of the other included stores (<tt>:active_record_store</tt>,
       # <tt>:mem_cache_store</tt>, or your own custom class.
       def session_store=(store)
-        if store == :active_record_store
-          self.session_store = ActiveRecord::SessionStore
-        else
-          @@session_store = store.is_a?(Symbol) ?
-            ActionDispatch::Session.const_get(store.to_s.camelize) :
-            store
-        end
+        ActiveSupport::Deprecation.warn "Setting session_store directly on ActionController::Base is deprecated. " \
+                                        "Please set it on config.action_controller.session_store"
+        config.session_store = store
+      end
+
+      def session_options=(opts)
+        ActiveSupport::Deprecation.warn "Setting seession_options directly on ActionController::Base is deprecated. " \
+                                        "Please set it on config.action_controller.session_options"
+        config.session_store = opts
+      end
+
+      def session_options
+        config.session_options
       end
 
-      # Returns the session store class currently used.
       def session_store
-        if defined? @@session_store
-          @@session_store
+        case store = config.session_store
+        when :active_record_store
+          ActiveRecord::SessionStore
+        when Symbol
+          ActionDispatch::Session.const_get(store.to_s.camelize)
         else
-          ActionDispatch::Session::CookieStore
+          store
         end
       end
 
diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb
index 7e9a2625f1..22d2e23de1 100644
--- a/actionpack/test/controller/http_digest_authentication_test.rb
+++ b/actionpack/test/controller/http_digest_authentication_test.rb
@@ -40,7 +40,8 @@ class HttpDigestAuthenticationTest < ActionController::TestCase
 
   setup do
     # Used as secret in generating nonce to prevent tampering of timestamp
-    @old_secret, ActionController::Base.session_options[:secret] = ActionController::Base.session_options[:secret], "session_options_secret"
+    @secret = "session_options_secret"
+    @old_secret, ActionController::Base.session_options[:secret] = ActionController::Base.session_options[:secret], @secret
   end
 
   teardown do
@@ -202,7 +203,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase
 
   test "validate_digest_response should fail with nil returning password_procedure" do
     @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => nil, :password => nil)
-    assert !ActionController::HttpAuthentication::Digest.validate_digest_response(@request, "SuperSecret"){nil}
+    assert !ActionController::HttpAuthentication::Digest.validate_digest_response(@secret, @request, "SuperSecret"){nil}
   end
 
   private

From 664090348154ccbf1274a13bbc3d3c37ba35bc7d Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Tue, 2 Mar 2010 17:18:01 -0800
Subject: [PATCH 02/19] Move InheritableOptions into ActiveSupport

---
 actionpack/lib/action_controller/metal.rb           | 8 --------
 activesupport/lib/active_support/ordered_options.rb | 6 ++++++
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb
index fbc8a9a15e..1f6bbeb7c5 100644
--- a/actionpack/lib/action_controller/metal.rb
+++ b/actionpack/lib/action_controller/metal.rb
@@ -1,14 +1,6 @@
 require 'active_support/core_ext/class/attribute'
 require 'active_support/ordered_options'
 
-module ActiveSupport
-  class InheritableOptions < OrderedOptions
-    def initialize(parent)
-      super() { |h,k| parent[k] }
-    end
-  end
-end
-
 module ActionController
   # ActionController::Metal provides a way to get a valid Rack application from a controller.
   #
diff --git a/activesupport/lib/active_support/ordered_options.rb b/activesupport/lib/active_support/ordered_options.rb
index 596a7b757d..61ccb79211 100644
--- a/activesupport/lib/active_support/ordered_options.rb
+++ b/activesupport/lib/active_support/ordered_options.rb
@@ -18,4 +18,10 @@ module ActiveSupport #:nodoc:
       end
     end
   end
+
+  class InheritableOptions < OrderedOptions
+    def initialize(parent)
+      super() { |h,k| parent[k] }
+    end
+  end
 end

From bcfb77782b9d7f28f0c19005da909162e5e27690 Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Tue, 2 Mar 2010 17:20:13 -0800
Subject: [PATCH 03/19] Work on deprecating
 ActionController::Base.relative_url_root

---
 .../action_controller/metal/compatibility.rb  | 41 ++++++++++++++++++-
 .../metal/session_management.rb               | 16 --------
 .../lib/action_controller/metal/url_for.rb    |  4 ++
 .../lib/action_controller/url_rewriter.rb     |  4 +-
 .../action_view/helpers/asset_tag_helper.rb   |  4 +-
 actionpack/test/controller/url_for_test.rb    | 15 ++-----
 actionpack/test/dispatch/request_test.rb      |  8 ----
 .../test/template/asset_tag_helper_test.rb    | 38 ++++++++---------
 8 files changed, 70 insertions(+), 60 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index d1c86b296d..93f7b8ca49 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -7,13 +7,17 @@ module ActionController
     class ::ActionController::ActionControllerError < StandardError #:nodoc:
     end
 
+    module ClassMethods
+    end
+
     # Temporary hax
     included do
       ::ActionController::UnknownAction = ::AbstractController::ActionNotFound
       ::ActionController::DoubleRenderError = ::AbstractController::DoubleRenderError
 
-      cattr_accessor :relative_url_root
-      self.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
+      # ROUTES TODO: This should be handled by a middleware and route generation
+      # should be able to handle SCRIPT_NAME
+      self.config.relative_url_root = ENV['RAILS_RELATIVE_URL_ROOT']
 
       class << self
         delegate :default_charset=, :to => "ActionDispatch::Response"
@@ -47,6 +51,39 @@ module ActionController
       cattr_accessor :trusted_proxies
     end
 
+    def self.deprecated_config_accessor(option, message = nil)
+      deprecated_config_reader(option, message)
+      deprecated_config_writer(option, message)
+    end
+
+    def self.deprecated_config_reader(option, message = nil)
+      message ||= "Reading #{option} directly from ActionController::Base is deprecated. " \
+                  "Please read it from config.#{option}"
+
+      ClassMethods.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+        def #{option}
+          ActiveSupport::Deprecation.warn #{message.inspect}
+          config.#{option}
+        end
+      RUBY
+    end
+
+    def self.deprecated_config_writer(option, message = nil)
+      message ||= "Setting #{option} directly on ActionController::Base is deprecated. " \
+                  "Please set it on config.action_controller.#{option}"
+
+      ClassMethods.class_eval <<-RUBY, __FILE__, __LINE__ + 1
+        def #{option}=(val)
+          ActiveSupport::Deprecation.warn #{message.inspect}
+          config.#{option} = val
+        end
+      RUBY
+    end
+
+    deprecated_config_writer :session_store
+    deprecated_config_writer :session_options
+    deprecated_config_accessor :relative_url_root, "relative_url_root is ineffective. Please stop using it"
+
     # For old tests
     def initialize_template_class(*) end
     def assign_shortcuts(*) end
diff --git a/actionpack/lib/action_controller/metal/session_management.rb b/actionpack/lib/action_controller/metal/session_management.rb
index 264250db1a..09ef9261a4 100644
--- a/actionpack/lib/action_controller/metal/session_management.rb
+++ b/actionpack/lib/action_controller/metal/session_management.rb
@@ -8,22 +8,6 @@ module ActionController #:nodoc:
     end
 
     module ClassMethods
-      # Set the session store to be used for keeping the session data between requests.
-      # By default, sessions are stored in browser cookies (<tt>:cookie_store</tt>),
-      # but you can also specify one of the other included stores (<tt>:active_record_store</tt>,
-      # <tt>:mem_cache_store</tt>, or your own custom class.
-      def session_store=(store)
-        ActiveSupport::Deprecation.warn "Setting session_store directly on ActionController::Base is deprecated. " \
-                                        "Please set it on config.action_controller.session_store"
-        config.session_store = store
-      end
-
-      def session_options=(opts)
-        ActiveSupport::Deprecation.warn "Setting seession_options directly on ActionController::Base is deprecated. " \
-                                        "Please set it on config.action_controller.session_options"
-        config.session_store = opts
-      end
-
       def session_options
         config.session_options
       end
diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb
index 8a06f34d23..c890dc51d4 100644
--- a/actionpack/lib/action_controller/metal/url_for.rb
+++ b/actionpack/lib/action_controller/metal/url_for.rb
@@ -9,6 +9,10 @@ module ActionController
       super.reverse_merge(
         :host => request.host_with_port,
         :protocol => request.protocol,
+        # ROUTES TODO: relative_url_root should be middleware
+        # and the generator should take SCRIPT_NAME into
+        # consideration
+        :relative_url_root => config.relative_url_root,
         :_path_segments => request.symbolized_path_parameters
       )
     end
diff --git a/actionpack/lib/action_controller/url_rewriter.rb b/actionpack/lib/action_controller/url_rewriter.rb
index 807b21cd0e..973a6facd7 100644
--- a/actionpack/lib/action_controller/url_rewriter.rb
+++ b/actionpack/lib/action_controller/url_rewriter.rb
@@ -32,6 +32,7 @@ module ActionController
 
       # ROUTES TODO: Fix the tests
       segments = options.delete(:_path_segments)
+      relative_url_root = options.delete(:relative_url_root).to_s
       path_segments = path_segments ? path_segments.merge(segments || {}) : segments
 
       unless options[:only_path]
@@ -49,7 +50,8 @@ module ActionController
       path_options = yield(path_options) if block_given?
       path = router.generate(path_options, path_segments || {})
 
-      rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
+      # ROUTES TODO: This can be called directly, so relative_url_root should probably be set in the router
+      rewritten_url << relative_url_root
       rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
       rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
 
diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
index 96976ce45f..5f76ff456e 100644
--- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
@@ -648,8 +648,8 @@ module ActionView
             source = rewrite_asset_path(source)
 
             if has_request && include_host
-              unless source =~ %r{^#{ActionController::Base.relative_url_root}/}
-                source = "#{ActionController::Base.relative_url_root}#{source}"
+              unless source =~ %r{^#{controller.config.relative_url_root}/}
+                source = "#{controller.config.relative_url_root}#{source}"
               end
             end
           end
diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb
index a7b77edc6e..07809aa480 100644
--- a/actionpack/test/controller/url_for_test.rb
+++ b/actionpack/test/controller/url_for_test.rb
@@ -113,15 +113,13 @@ module AbstractController
       end
 
       def test_relative_url_root_is_respected
-        orig_relative_url_root = ActionController::Base.relative_url_root
-        ActionController::Base.relative_url_root = '/subdir'
+        # ROUTES TODO: Tests should not have to pass :relative_url_root directly. This
+        # should probably come from the router.
 
         add_host!
         assert_equal('https://www.basecamphq.com/subdir/c/a/i',
-          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https')
+          W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => 'https', :relative_url_root => '/subdir')
         )
-      ensure
-        ActionController::Base.relative_url_root = orig_relative_url_root
       end
 
       def test_named_routes
@@ -146,9 +144,6 @@ module AbstractController
       end
 
       def test_relative_url_root_is_respected_for_named_routes
-        orig_relative_url_root = ActionController::Base.relative_url_root
-        ActionController::Base.relative_url_root = '/subdir'
-
         with_routing do |set|
           set.draw do |map|
             match '/home/sweet/home/:user', :to => 'home#index', :as => :home
@@ -158,10 +153,8 @@ module AbstractController
           controller = kls.new
 
           assert_equal 'http://www.basecamphq.com/subdir/home/sweet/home/again',
-            controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again')
+            controller.send(:home_url, :host => 'www.basecamphq.com', :user => 'again', :relative_url_root => "/subdir")
         end
-      ensure
-        ActionController::Base.relative_url_root = orig_relative_url_root
       end
 
       def test_only_path
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index cc6acead6e..703f03fa96 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -1,14 +1,6 @@
 require 'abstract_unit'
 
 class RequestTest < ActiveSupport::TestCase
-  def setup
-    ActionController::Base.relative_url_root = nil
-  end
-
-  def teardown
-    ActionController::Base.relative_url_root = nil
-  end
-
   test "remote ip" do
     request = stub_request 'REMOTE_ADDR' => '1.2.3.4'
     assert_equal '1.2.3.4', request.remote_ip
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index 586de66714..ccc39e9af6 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -1,4 +1,13 @@
 require 'abstract_unit'
+require 'active_support/ordered_options'
+
+class FakeController
+  attr_accessor :request
+
+  def config
+    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Metal.config)
+  end
+end
 
 class AssetTagHelperTest < ActionView::TestCase
   tests ActionView::Helpers::AssetTagHelper
@@ -32,8 +41,7 @@ class AssetTagHelperTest < ActionView::TestCase
       )
     end
 
-    @controller = Class.new do
-      attr_accessor :request
+    @controller = Class.new(FakeController) do
       def url_for(*args) "http://www.example.com" end
     end.new
 
@@ -372,11 +380,9 @@ class AssetTagHelperTest < ActionView::TestCase
   end
 
   def test_timebased_asset_id_with_relative_url_root
-      ActionController::Base.relative_url_root = "/collaboration/hieraki"
-      expected_time = File.stat(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).mtime.to_i.to_s
-      assert_equal %(<img alt="Rails" src="#{ActionController::Base.relative_url_root}/images/rails.png?#{expected_time}" />), image_tag("rails.png")
-    ensure
-      ActionController::Base.relative_url_root = ""
+    @controller.config.relative_url_root = "/collaboration/hieraki"
+    expected_time = File.stat(File.expand_path(File.dirname(__FILE__) + "/../fixtures/public/images/rails.png")).mtime.to_i.to_s
+    assert_equal %(<img alt="Rails" src="#{@controller.config.relative_url_root}/images/rails.png?#{expected_time}" />), image_tag("rails.png")
   end
 
   def test_should_skip_asset_id_on_complete_url
@@ -606,7 +612,7 @@ class AssetTagHelperTest < ActionView::TestCase
 
   def test_caching_javascript_include_tag_with_relative_url_root
     ENV["RAILS_ASSET_ID"] = ""
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
+    @controller.config.relative_url_root = "/collaboration/hieraki"
     ActionController::Base.perform_caching = true
 
     assert_dom_equal(
@@ -624,7 +630,6 @@ class AssetTagHelperTest < ActionView::TestCase
     assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js'))
 
   ensure
-    ActionController::Base.relative_url_root = nil
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'all.js'))
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'money.js'))
   end
@@ -821,7 +826,7 @@ class AssetTagHelperTest < ActionView::TestCase
 
   def test_caching_stylesheet_link_tag_with_relative_url_root
     ENV["RAILS_ASSET_ID"] = ""
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
+    @controller.config.relative_url_root = "/collaboration/hieraki"
     ActionController::Base.perform_caching = true
 
     assert_dom_equal(
@@ -841,7 +846,6 @@ class AssetTagHelperTest < ActionView::TestCase
 
     assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css'))
   ensure
-    ActionController::Base.relative_url_root = nil
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'all.css'))
     FileUtils.rm_f(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css'))
   end
@@ -884,16 +888,14 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase
 
   def setup
     super
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
-
-    @controller = Class.new do
-      attr_accessor :request
-
+    @controller = Class.new(FakeController) do
       def url_for(options)
         "http://www.example.com/collaboration/hieraki"
       end
     end.new
 
+    @controller.config.relative_url_root = "/collaboration/hieraki"
+
     @request = Class.new do
       def protocol
         'gopher://'
@@ -905,10 +907,6 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase
     ActionView::Helpers::AssetTagHelper::reset_javascript_include_default
   end
 
-  def teardown
-    ActionController::Base.relative_url_root = nil
-  end
-
   def test_should_compute_proper_path
     assert_dom_equal(%(<link href="http://www.example.com/collaboration/hieraki" rel="alternate" title="RSS" type="application/rss+xml" />), auto_discovery_link_tag)
     assert_dom_equal(%(/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr"))

From 5e0a05b8cb236d285ebb45de006dd3600c69357d Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Tue, 2 Mar 2010 18:57:02 -0800
Subject: [PATCH 04/19] Tweak the semantic of various URL related methods of
 ActionDispatch::Request

---
 actionpack/lib/action_controller/test_case.rb | 22 +++--
 actionpack/lib/action_dispatch/http/url.rb    | 31 ++-----
 .../lib/action_view/helpers/url_helper.rb     |  5 +-
 actionpack/lib/action_view/test_case.rb       |  2 +
 .../http_digest_authentication_test.rb        | 10 ++-
 .../test/controller/integration_test.rb       |  4 +-
 actionpack/test/dispatch/request_test.rb      | 86 ++-----------------
 actionpack/test/template/url_helper_test.rb   |  3 -
 8 files changed, 41 insertions(+), 122 deletions(-)

diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index 64d9bdab2a..7e0a833dfa 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -322,6 +322,8 @@ module ActionController
         @controller ||= klass.new rescue nil
       end
 
+      @request.env.delete('PATH_INFO')
+
       if @controller
         @controller.request = @request
         @controller.params = {}
@@ -333,15 +335,19 @@ module ActionController
       @request.remote_addr = '208.77.188.166' # example.com
     end
 
-    private
-      def build_request_uri(action, parameters)
-        unless @request.env['REQUEST_URI']
-          options = @controller.__send__(:url_options).merge(parameters)
-          options.update(:only_path => true, :action => action)
+  private
+    def build_request_uri(action, parameters)
+      unless @request.env["PATH_INFO"]
+        options = @controller.__send__(:url_options).merge(parameters)
+        options.update(:only_path => true, :action => action, :relative_url_root => nil)
+        rewriter = ActionController::UrlRewriter.new(@request, parameters)
 
-          url = ActionController::UrlRewriter.new(@request, parameters)
-          @request.request_uri = url.rewrite(@router, options)
-        end
+        url, query_string = rewriter.rewrite(@router, options).split("?", 2)
+
+        @request.env["SCRIPT_NAME"] = @controller.config.relative_url_root
+        @request.env["PATH_INFO"] = url
+        @request.env["QUERY_STRING"] = query_string || ""
       end
+    end
   end
 end
diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 42ad19044d..e67d970a23 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -85,42 +85,23 @@ module ActionDispatch
         subdomains(tld_length).join('.')
       end
 
-      # Returns the query string, accounting for server idiosyncrasies.
+      # The query string must always be present
       def query_string
-        @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].to_s.split('?', 2)[1] || '')
+        @env['QUERY_STRING']
       end
 
       # Returns the request URI, accounting for server idiosyncrasies.
       # WEBrick includes the full URL. IIS leaves REQUEST_URI blank.
       def request_uri
-        if uri = @env['REQUEST_URI']
-          # Remove domain, which webrick puts into the request_uri.
-          (%r{^\w+\://[^/]+(/.*|$)$} =~ uri) ? $1 : uri
-        else
-          # Construct IIS missing REQUEST_URI from SCRIPT_NAME and PATH_INFO.
-          uri = @env['PATH_INFO'].to_s
-
-          if script_filename = @env['SCRIPT_NAME'].to_s.match(%r{[^/]+$})
-            uri = uri.sub(/#{script_filename}\//, '')
-          end
-
-          env_qs = @env['QUERY_STRING'].to_s
-          uri += "?#{env_qs}" unless env_qs.empty?
-
-          if uri.blank?
-            @env.delete('REQUEST_URI')
-          else
-            @env['REQUEST_URI'] = uri
-          end
-        end
+        uri = "#{@env["SCRIPT_NAME"]}#{@env["PATH_INFO"]}"
+        uri << "?#{@env["QUERY_STRING"]}" if @env["QUERY_STRING"].present?
+        uri
       end
 
       # Returns the interpreted \path to requested resource after all the installation
       # directory of this application was taken into account.
       def path
-        path = request_uri.to_s[/\A[^\?]*/]
-        path.sub!(/\A#{ActionController::Base.relative_url_root}/, '')
-        path
+        @env['PATH_INFO']
       end
 
     private
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index d9607c0095..148f2868e9 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -544,10 +544,11 @@ module ActionView
         # submitted url doesn't have any either.  This lets the function
         # work with things like ?order=asc
         if url_string.index("?")
-          request_uri = request.request_uri
+          request_uri = request.fullpath
         else
-          request_uri = request.request_uri.split('?').first
+          request_uri = request.path
         end
+
         if url_string =~ /^\w+:\/\//
           url_string == "#{request.protocol}#{request.host_with_port}#{request_uri}"
         else
diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb
index f639700eaf..12142a3728 100644
--- a/actionpack/lib/action_view/test_case.rb
+++ b/actionpack/lib/action_view/test_case.rb
@@ -34,6 +34,8 @@ module ActionView
         @request = ActionController::TestRequest.new
         @response = ActionController::TestResponse.new
 
+        @request.env.delete('PATH_INFO')
+
         @params = {}
       end
     end
diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb
index 22d2e23de1..6f167fe627 100644
--- a/actionpack/test/controller/http_digest_authentication_test.rb
+++ b/actionpack/test/controller/http_digest_authentication_test.rb
@@ -139,7 +139,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase
 
   test "authentication request with request-uri that doesn't match credentials digest-uri" do
     @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please')
-    @request.env['REQUEST_URI'] = "/http_digest_authentication_test/dummy_digest/altered/uri"
+    @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest/altered/uri"
     get :display
 
     assert_response :unauthorized
@@ -148,7 +148,8 @@ class HttpDigestAuthenticationTest < ActionController::TestCase
 
   test "authentication request with absolute request uri (as in webrick)" do
     @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:username => 'pretty', :password => 'please')
-    @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest"
+    @request.env["SERVER_NAME"] = "test.host"
+    @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest"
 
     get :display
 
@@ -171,7 +172,8 @@ class HttpDigestAuthenticationTest < ActionController::TestCase
   test "authentication request with absolute uri in both request and credentials (as in Webrick with IE)" do
     @request.env['HTTP_AUTHORIZATION'] = encode_credentials(:url => "http://test.host/http_digest_authentication_test/dummy_digest",
                                                             :username => 'pretty', :password => 'please')
-    @request.env['REQUEST_URI'] = "http://test.host/http_digest_authentication_test/dummy_digest"
+    @request.env['SERVER_NAME'] = "test.host"
+    @request.env['PATH_INFO'] = "/http_digest_authentication_test/dummy_digest"
 
     get :display
 
@@ -226,7 +228,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase
 
     credentials = decode_credentials(@response.headers['WWW-Authenticate'])
     credentials.merge!(options)
-    credentials.merge!(:uri => @request.env['REQUEST_URI'].to_s)
+    credentials.merge!(:uri => @request.env['PATH_INFO'].to_s)
     ActionController::HttpAuthentication::Digest.encode_credentials(method, credentials, password, options[:password_is_ha1])
   end
 
diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb
index c38348fa68..814699978d 100644
--- a/actionpack/test/controller/integration_test.rb
+++ b/actionpack/test/controller/integration_test.rb
@@ -346,8 +346,8 @@ class IntegrationProcessTest < ActionController::IntegrationTest
   def test_get_with_parameters
     with_test_route_set do
       get '/get_with_params', :foo => "bar"
-      assert_equal '/get_with_params', request.env["REQUEST_URI"]
-      assert_equal '/get_with_params', request.request_uri
+      assert_equal '/get_with_params', request.env["PATH_INFO"]
+      assert_equal '/get_with_params', request.path_info
       assert_equal 'foo=bar', request.env["QUERY_STRING"]
       assert_equal 'foo=bar', request.query_string
       assert_equal 'bar', request.parameters['foo']
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 703f03fa96..1dfcdaebfd 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -144,103 +144,33 @@ class RequestTest < ActiveSupport::TestCase
   end
 
   test "request uri" do
-    request = stub_request 'REQUEST_URI' => "http://www.rubyonrails.org/path/of/some/uri?mapped=1"
+    request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/path/of/some/uri', 'QUERY_STRING' => 'mapped=1'
     assert_equal "/path/of/some/uri?mapped=1", request.request_uri
     assert_equal "/path/of/some/uri",          request.path
 
-    request = stub_request 'REQUEST_URI' => "http://www.rubyonrails.org/path/of/some/uri"
+    request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/path/of/some/uri'
     assert_equal "/path/of/some/uri", request.request_uri
     assert_equal "/path/of/some/uri", request.path
 
-    request = stub_request 'REQUEST_URI' => "/path/of/some/uri"
-    assert_equal "/path/of/some/uri", request.request_uri
-    assert_equal "/path/of/some/uri", request.path
-
-    request = stub_request 'REQUEST_URI' => "/"
+    request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/'
     assert_equal "/", request.request_uri
     assert_equal "/", request.path
 
-    request = stub_request 'REQUEST_URI' => "/?m=b"
+    request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/', 'QUERY_STRING' => 'm=b'
     assert_equal "/?m=b", request.request_uri
     assert_equal "/",     request.path
 
-    request = stub_request 'REQUEST_URI' => "/", 'SCRIPT_NAME' => '/dispatch.cgi'
-    assert_equal "/", request.request_uri
-    assert_equal "/", request.path
-
-    ActionController::Base.relative_url_root = "/hieraki"
-    request = stub_request 'REQUEST_URI' => "/hieraki/", 'SCRIPT_NAME' => "/hieraki/dispatch.cgi"
+    request = stub_request 'SCRIPT_NAME' => '/hieraki', 'PATH_INFO' => '/'
     assert_equal "/hieraki/", request.request_uri
     assert_equal "/",         request.path
-    ActionController::Base.relative_url_root = nil
 
-    ActionController::Base.relative_url_root = "/collaboration/hieraki"
-    request = stub_request 'REQUEST_URI' => "/collaboration/hieraki/books/edit/2",
-      'SCRIPT_NAME' => "/collaboration/hieraki/dispatch.cgi"
+    request = stub_request 'SCRIPT_NAME' => '/collaboration/hieraki', 'PATH_INFO' => '/books/edit/2'
     assert_equal "/collaboration/hieraki/books/edit/2", request.request_uri
     assert_equal "/books/edit/2",                       request.path
-    ActionController::Base.relative_url_root = nil
 
-    # The following tests are for when REQUEST_URI is not supplied (as in IIS)
-    request = stub_request 'PATH_INFO'   => "/path/of/some/uri?mapped=1",
-                           'SCRIPT_NAME' => nil,
-                           'REQUEST_URI' => nil
-    assert_equal "/path/of/some/uri?mapped=1", request.request_uri
-    assert_equal "/path/of/some/uri",          request.path
-
-    ActionController::Base.relative_url_root = '/path'
-    request = stub_request 'PATH_INFO'   => "/path/of/some/uri?mapped=1",
-                           'SCRIPT_NAME' => "/path/dispatch.rb",
-                           'REQUEST_URI' => nil
+    request = stub_request 'SCRIPT_NAME' => '/path', 'PATH_INFO' => '/of/some/uri', 'QUERY_STRING' => 'mapped=1'
     assert_equal "/path/of/some/uri?mapped=1", request.request_uri
     assert_equal "/of/some/uri",               request.path
-    ActionController::Base.relative_url_root = nil
-
-    request = stub_request 'PATH_INFO'   => "/path/of/some/uri",
-                           'SCRIPT_NAME' => nil,
-                           'REQUEST_URI' => nil
-    assert_equal "/path/of/some/uri", request.request_uri
-    assert_equal "/path/of/some/uri", request.path
-
-    request = stub_request 'PATH_INFO' => '/', 'REQUEST_URI' => nil
-    assert_equal "/", request.request_uri
-    assert_equal "/", request.path
-
-    request = stub_request 'PATH_INFO' => '/?m=b', 'REQUEST_URI' => nil
-    assert_equal "/?m=b", request.request_uri
-    assert_equal "/",     request.path
-
-    request = stub_request 'PATH_INFO'   => "/",
-                           'SCRIPT_NAME' => "/dispatch.cgi",
-                           'REQUEST_URI' => nil
-    assert_equal "/", request.request_uri
-    assert_equal "/", request.path
-
-    ActionController::Base.relative_url_root = '/hieraki'
-    request = stub_request 'PATH_INFO'   => "/hieraki/",
-                           'SCRIPT_NAME' => "/hieraki/dispatch.cgi",
-                           'REQUEST_URI' => nil
-    assert_equal "/hieraki/", request.request_uri
-    assert_equal "/",         request.path
-    ActionController::Base.relative_url_root = nil
-
-    request = stub_request 'REQUEST_URI' => '/hieraki/dispatch.cgi'
-    ActionController::Base.relative_url_root = '/hieraki'
-    assert_equal "/dispatch.cgi", request.path
-    ActionController::Base.relative_url_root = nil
-
-    request = stub_request 'REQUEST_URI' => '/hieraki/dispatch.cgi'
-    ActionController::Base.relative_url_root = '/foo'
-    assert_equal "/hieraki/dispatch.cgi", request.path
-    ActionController::Base.relative_url_root = nil
-
-    # This test ensures that Rails uses REQUEST_URI over PATH_INFO
-    ActionController::Base.relative_url_root = nil
-    request = stub_request 'REQUEST_URI' => "/some/path",
-                           'PATH_INFO'   => "/another/path",
-                           'SCRIPT_NAME' => "/dispatch.cgi"
-    assert_equal "/some/path", request.request_uri
-    assert_equal "/some/path", request.path
   end
 
 
@@ -498,7 +428,7 @@ class RequestTest < ActiveSupport::TestCase
 
 protected
 
-  def stub_request(env={})
+  def stub_request(env = {})
     ActionDispatch::Request.new(env)
   end
 
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index b047466aaf..afec78ddce 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -464,8 +464,6 @@ end
 class LinkToUnlessCurrentWithControllerTest < ActionController::TestCase
   def setup
     super
-    @request    = ActionController::TestRequest.new
-    @response   = ActionController::TestResponse.new
     @controller = TasksController.new
   end
 
@@ -565,7 +563,6 @@ end
 class PolymorphicControllerTest < ActionController::TestCase
   def setup
     super
-    @request  = ActionController::TestRequest.new
     @response = ActionController::TestResponse.new
   end
 

From c92d598abff54270337ad1eddf7bc41a71c4cbda Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 00:03:29 -0800
Subject: [PATCH 05/19] Rack::Request actually defines #query_string

---
 actionpack/lib/action_dispatch/http/url.rb | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index e67d970a23..049e1758fe 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -85,11 +85,6 @@ module ActionDispatch
         subdomains(tld_length).join('.')
       end
 
-      # The query string must always be present
-      def query_string
-        @env['QUERY_STRING']
-      end
-
       # Returns the request URI, accounting for server idiosyncrasies.
       # WEBrick includes the full URL. IIS leaves REQUEST_URI blank.
       def request_uri

From eb49bd694997954783632eada901553f041c9507 Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 00:03:56 -0800
Subject: [PATCH 06/19] Fix tests for the request refactor

---
 actionpack/test/controller/caching_test.rb  |  3 +-
 actionpack/test/controller/test_test.rb     |  6 ++-
 actionpack/test/template/url_helper_test.rb | 52 ++++++++++++++-------
 3 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 80c968cc04..37f7483427 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -51,6 +51,7 @@ class PageCachingTest < ActionController::TestCase
 
     @request = ActionController::TestRequest.new
     @request.host = 'hostname.com'
+    @request.env.delete('PATH_INFO')
 
     @response   = ActionController::TestResponse.new
     @controller = PageCachingTestController.new
@@ -110,7 +111,7 @@ class PageCachingTest < ActionController::TestCase
   end
 
   def test_should_cache_ok_at_custom_path
-    @request.request_uri = "/index.html"
+    @request.env['PATH_INFO'] = '/index.html'
     get :ok
     assert_response :ok
     assert File.exist?("#{FILE_STORE_PATH}/index.html")
diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb
index d716dc2661..7312c5dfcc 100644
--- a/actionpack/test/controller/test_test.rb
+++ b/actionpack/test/controller/test_test.rb
@@ -128,6 +128,7 @@ XML
     @controller = TestController.new
     @request    = ActionController::TestRequest.new
     @response   = ActionController::TestResponse.new
+    @request.env['PATH_INFO'] = nil
   end
 
   def test_raw_post_handling
@@ -199,7 +200,7 @@ XML
   end
 
   def test_process_with_request_uri_with_params_with_explicit_uri
-    @request.request_uri = "/explicit/uri"
+    @request.env['PATH_INFO'] = "/explicit/uri"
     process :test_uri, :id => 7
     assert_equal "/explicit/uri", @response.body
   end
@@ -210,7 +211,8 @@ XML
   end
 
   def test_process_with_query_string_with_explicit_uri
-    @request.request_uri = "/explicit/uri?q=test?extra=question"
+    @request.env['PATH_INFO'] = '/explicit/uri'
+    @request.env['QUERY_STRING'] = 'q=test?extra=question'
     process :test_query_string
     assert_equal "q=test?extra=question", @response.body
   end
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index afec78ddce..d8ccb380a6 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -1,9 +1,8 @@
 # encoding: utf-8
 require 'abstract_unit'
+require 'active_support/ordered_options'
 require 'controller/fake_controllers'
 
-RequestMock = Struct.new("Request", :request_uri, :protocol, :host_with_port, :env)
-
 class UrlHelperTest < ActionView::TestCase
   include ActiveSupport::Configurable
   DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG
@@ -15,8 +14,13 @@ class UrlHelperTest < ActionView::TestCase
       def url_for(options)
         url
       end
+      def config
+        ActiveSupport::InheritableOptions.new({})
+      end
     end
+
     @controller = @controller.new
+    @request = @controller.request = ActionDispatch::TestRequest.new
     @controller.url = "http://www.example.com"
   end
 
@@ -38,12 +42,13 @@ class UrlHelperTest < ActionView::TestCase
   end
 
   def test_url_for_with_back
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show", nil, nil, {'HTTP_REFERER' => 'http://www.example.com/referer'})
+    @request.env['HTTP_REFERER'] = 'http://www.example.com/referer'
     assert_equal 'http://www.example.com/referer', url_for(:back)
   end
 
   def test_url_for_with_back_and_no_referer
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show", nil, nil, {})
+    @request.env['HOST_NAME'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
     assert_equal 'javascript:history.back()', url_for(:back)
   end
 
@@ -144,22 +149,28 @@ class UrlHelperTest < ActionView::TestCase
   end
 
   def test_link_tag_with_back
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show", nil, nil, {'HTTP_REFERER' => 'http://www.example.com/referer'})
+    @request.env['HOST_NAME'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
+    @request.env['HTTP_REFERER'] = 'http://www.example.com/referer'
     assert_dom_equal "<a href=\"http://www.example.com/referer\">go back</a>", link_to('go back', :back)
   end
 
   def test_link_tag_with_back_and_no_referer
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show", nil, nil, {})
+    @request.env['HOST_NAME'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
     assert_dom_equal "<a href=\"javascript:history.back()\">go back</a>", link_to('go back', :back)
   end
 
   def test_link_tag_with_back
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show", nil, nil, {'HTTP_REFERER' => 'http://www.example.com/referer'})
+    @request.env['HOST_NAME'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
+    @request.env['HTTP_REFERER'] = 'http://www.example.com/referer'
     assert_dom_equal "<a href=\"http://www.example.com/referer\">go back</a>", link_to('go back', :back)
   end
 
   def test_link_tag_with_back_and_no_referer
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show", nil, nil, {})
+    @request.env['HOST_NAME'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
     assert_dom_equal "<a href=\"javascript:history.back()\">go back</a>", link_to('go back', :back)
   end
 
@@ -263,55 +274,60 @@ class UrlHelperTest < ActionView::TestCase
   end
 
   def test_current_page_with_simple_url
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show")
+    @request.env['HTTP_HOST'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
     @controller.url = "http://www.example.com/weblog/show"
     assert current_page?({ :action => "show", :controller => "weblog" })
     assert current_page?("http://www.example.com/weblog/show")
   end
 
   def test_current_page_ignoring_params
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1")
+    @request.env['HTTP_HOST'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
+    @request.env['QUERY_STRING'] = 'order=desc&page=1'
     @controller.url = "http://www.example.com/weblog/show?order=desc&page=1"
     assert current_page?({ :action => "show", :controller => "weblog" })
     assert current_page?("http://www.example.com/weblog/show")
   end
 
   def test_current_page_with_params_that_match
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1")
+    @request.env['HTTP_HOST'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
+    @request.env['QUERY_STRING'] = 'order=desc&page=1'
     @controller.url = "http://www.example.com/weblog/show?order=desc&page=1"
     assert current_page?({ :action => "show", :controller => "weblog", :order => "desc", :page => "1" })
     assert current_page?("http://www.example.com/weblog/show?order=desc&amp;page=1")
   end
 
   def test_link_unless_current
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show")
+    @request.env['HTTP_HOST'] = 'www.example.com'
+    @request.env['PATH_INFO'] = '/weblog/show'
     @controller.url = "http://www.example.com/weblog/show"
     assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
     assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show")
 
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc")
+    @request.env['QUERY_STRING'] = 'order=desc'
     @controller.url = "http://www.example.com/weblog/show"
     assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
     assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show")
 
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1")
+    @request.env['QUERY_STRING'] = 'order=desc&page=1'
     @controller.url = "http://www.example.com/weblog/show?order=desc&page=1"
     assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog", :order=>'desc', :page=>'1' })
     assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1")
     assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1")
 
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc")
+    @request.env['QUERY_STRING'] = 'order=desc'
     @controller.url = "http://www.example.com/weblog/show?order=asc"
     assert_equal "<a href=\"http://www.example.com/weblog/show?order=asc\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
     assert_equal "<a href=\"http://www.example.com/weblog/show?order=asc\">Showing</a>", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=asc")
 
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1")
+    @request.env['QUERY_STRING'] = 'order=desc&page=1'
     @controller.url = "http://www.example.com/weblog/show?order=desc&page=2"
     assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&page=2\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
     assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&amp;page=2\">Showing</a>", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=2")
 
-
-    @controller.request = RequestMock.new("http://www.example.com/weblog/show")
+    @request.env['QUERY_STRING'] = ''
     @controller.url = "http://www.example.com/weblog/list"
     assert_equal "<a href=\"http://www.example.com/weblog/list\">Listing</a>",
       link_to_unless_current("Listing", :action => "list", :controller => "weblog")

From fb14b8c6fddae818b2688ac1e584534390c37f72 Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 00:31:55 -0800
Subject: [PATCH 07/19] ActionDispatch::Request deprecates #request_uri

	* Refactored ActionPatch to use fullpath instead
---
 .../metal/http_authentication.rb              |  2 +-
 .../metal/instrumentation.rb                  |  2 +-
 actionpack/lib/action_dispatch/http/url.rb    | 13 ++------
 .../action_view/helpers/atom_feed_helper.rb   |  2 +-
 .../test/controller/integration_test.rb       |  2 +-
 actionpack/test/controller/test_test.rb       |  2 +-
 actionpack/test/dispatch/request_test.rb      | 30 +++++++++----------
 7 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb
index 5c6641c948..afa7674e40 100644
--- a/actionpack/lib/action_controller/metal/http_authentication.rb
+++ b/actionpack/lib/action_controller/metal/http_authentication.rb
@@ -198,7 +198,7 @@ module ActionController
           return false unless password
 
           method = request.env['rack.methodoverride.original_method'] || request.env['REQUEST_METHOD']
-          uri    = credentials[:uri][0,1] == '/' ? request.request_uri : request.url
+          uri    = credentials[:uri][0,1] == '/' ? request.fullpath : request.url
 
          [true, false].any? do |password_is_ha1|
            expected = expected_response(method, uri, credentials, password, password_is_ha1)
diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb
index 85035dc09c..d69de65f28 100644
--- a/actionpack/lib/action_controller/metal/instrumentation.rb
+++ b/actionpack/lib/action_controller/metal/instrumentation.rb
@@ -20,7 +20,7 @@ module ActionController
         :params     => request.filtered_parameters,
         :formats    => request.formats.map(&:to_sym),
         :method     => request.method,
-        :path       => (request.request_uri rescue "unknown")
+        :path       => (request.fullpath rescue "unknown")
       }
 
       ActiveSupport::Notifications.instrument("action_controller.start_processing", raw_payload.dup)
diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 049e1758fe..f7afbe7fed 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -3,7 +3,7 @@ module ActionDispatch
     module URL
       # Returns the complete URL used for this request.
       def url
-        protocol + host_with_port + request_uri
+        protocol + host_with_port + fullpath
       end
 
       # Returns 'https://' if this is an SSL request and 'http://' otherwise.
@@ -88,15 +88,8 @@ module ActionDispatch
       # Returns the request URI, accounting for server idiosyncrasies.
       # WEBrick includes the full URL. IIS leaves REQUEST_URI blank.
       def request_uri
-        uri = "#{@env["SCRIPT_NAME"]}#{@env["PATH_INFO"]}"
-        uri << "?#{@env["QUERY_STRING"]}" if @env["QUERY_STRING"].present?
-        uri
-      end
-
-      # Returns the interpreted \path to requested resource after all the installation
-      # directory of this application was taken into account.
-      def path
-        @env['PATH_INFO']
+        ActiveSupport::Deprecation.warn "Using #request_uri is deprecated. Use fullpath instead."
+        fullpath
       end
 
     private
diff --git a/actionpack/lib/action_view/helpers/atom_feed_helper.rb b/actionpack/lib/action_view/helpers/atom_feed_helper.rb
index a26a8c9b4b..58c3a8752e 100644
--- a/actionpack/lib/action_view/helpers/atom_feed_helper.rb
+++ b/actionpack/lib/action_view/helpers/atom_feed_helper.rb
@@ -114,7 +114,7 @@ module ActionView
         feed_opts.merge!(options).reject!{|k,v| !k.to_s.match(/^xml/)}
 
         xml.feed(feed_opts) do
-          xml.id(options[:id] || "tag:#{request.host},#{options[:schema_date]}:#{request.request_uri.split(".")[0]}")
+          xml.id(options[:id] || "tag:#{request.host},#{options[:schema_date]}:#{request.fullpath.split(".")[0]}")
           xml.link(:rel => 'alternate', :type => 'text/html', :href => options[:root_url] || (request.protocol + request.host_with_port))
           xml.link(:rel => 'self', :type => 'application/atom+xml', :href => options[:url] || request.url)
 
diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb
index 814699978d..5cb6aa6997 100644
--- a/actionpack/test/controller/integration_test.rb
+++ b/actionpack/test/controller/integration_test.rb
@@ -333,7 +333,7 @@ class IntegrationProcessTest < ActionController::IntegrationTest
     with_test_route_set do
       get '/get_with_params?foo=bar'
       assert_equal '/get_with_params?foo=bar', request.env["REQUEST_URI"]
-      assert_equal '/get_with_params?foo=bar', request.request_uri
+      assert_equal '/get_with_params?foo=bar', request.fullpath
       assert_equal "foo=bar", request.env["QUERY_STRING"]
       assert_equal 'foo=bar', request.query_string
       assert_equal 'bar', request.parameters['foo']
diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb
index 7312c5dfcc..f6ba275849 100644
--- a/actionpack/test/controller/test_test.rb
+++ b/actionpack/test/controller/test_test.rb
@@ -42,7 +42,7 @@ class TestTest < ActionController::TestCase
     end
 
     def test_uri
-      render :text => request.request_uri
+      render :text => request.fullpath
     end
 
     def test_query_string
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 1dfcdaebfd..84c06ca113 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -143,34 +143,34 @@ class RequestTest < ActiveSupport::TestCase
     assert_equal ":8080", request.port_string
   end
 
-  test "request uri" do
+  test "full path" do
     request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/path/of/some/uri', 'QUERY_STRING' => 'mapped=1'
-    assert_equal "/path/of/some/uri?mapped=1", request.request_uri
-    assert_equal "/path/of/some/uri",          request.path
+    assert_equal "/path/of/some/uri?mapped=1", request.fullpath
+    assert_equal "/path/of/some/uri",          request.path_info
 
     request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/path/of/some/uri'
-    assert_equal "/path/of/some/uri", request.request_uri
-    assert_equal "/path/of/some/uri", request.path
+    assert_equal "/path/of/some/uri", request.fullpath
+    assert_equal "/path/of/some/uri", request.path_info
 
     request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/'
-    assert_equal "/", request.request_uri
-    assert_equal "/", request.path
+    assert_equal "/", request.fullpath
+    assert_equal "/", request.path_info
 
     request = stub_request 'SCRIPT_NAME' => '', 'PATH_INFO' => '/', 'QUERY_STRING' => 'm=b'
-    assert_equal "/?m=b", request.request_uri
-    assert_equal "/",     request.path
+    assert_equal "/?m=b", request.fullpath
+    assert_equal "/",     request.path_info
 
     request = stub_request 'SCRIPT_NAME' => '/hieraki', 'PATH_INFO' => '/'
-    assert_equal "/hieraki/", request.request_uri
-    assert_equal "/",         request.path
+    assert_equal "/hieraki/", request.fullpath
+    assert_equal "/",         request.path_info
 
     request = stub_request 'SCRIPT_NAME' => '/collaboration/hieraki', 'PATH_INFO' => '/books/edit/2'
-    assert_equal "/collaboration/hieraki/books/edit/2", request.request_uri
-    assert_equal "/books/edit/2",                       request.path
+    assert_equal "/collaboration/hieraki/books/edit/2", request.fullpath
+    assert_equal "/books/edit/2",                       request.path_info
 
     request = stub_request 'SCRIPT_NAME' => '/path', 'PATH_INFO' => '/of/some/uri', 'QUERY_STRING' => 'mapped=1'
-    assert_equal "/path/of/some/uri?mapped=1", request.request_uri
-    assert_equal "/of/some/uri",               request.path
+    assert_equal "/path/of/some/uri?mapped=1", request.fullpath
+    assert_equal "/of/some/uri",               request.path_info
   end
 
 

From 18bcce596efaa4e77a202431ab5e736001a394c6 Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 09:32:27 -0800
Subject: [PATCH 08/19] ActionController::Base.use_accept_header is not
 actually used anymore, so let's deprecate it.

---
 .../lib/action_controller/metal/compatibility.rb     | 10 ++++++++--
 actionpack/test/controller/caching_test.rb           |  3 ---
 actionpack/test/controller/content_type_test.rb      | 12 ------------
 actionpack/test/controller/mime_responds_test.rb     |  4 ----
 actionpack/test/dispatch/request_test.rb             |  7 -------
 5 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 93f7b8ca49..d6d8d612a3 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -35,8 +35,14 @@ module ActionController
       cattr_accessor :resource_action_separator
       self.resource_action_separator = "/"
 
-      cattr_accessor :use_accept_header
-      self.use_accept_header = true
+      def self.use_accept_header
+        ActiveSupport::Deprecation.warn "ActionController::Base.use_accept_header doesn't do anything anymore. " \
+                                        "The accept header is always taken into account."
+      end
+
+      def self.use_accept_header=(val)
+        use_accept_header
+      end
 
       self.page_cache_directory = defined?(Rails.public_path) ? Rails.public_path : ""
 
diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 37f7483427..e42ef85b98 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -306,12 +306,9 @@ class ActionCacheTest < ActionController::TestCase
   end
 
   def test_action_cache_conditional_options
-    old_use_accept_header = ActionController::Base.use_accept_header
-    ActionController::Base.use_accept_header = true
     @request.env['HTTP_ACCEPT'] = 'application/json'
     get :index
     assert !fragment_exist?('hostname.com/action_caching_test')
-    ActionController::Base.use_accept_header = old_use_accept_header
   end
 
   def test_action_cache_with_store_options
diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb
index e5ffe20ecc..967107853b 100644
--- a/actionpack/test/controller/content_type_test.rb
+++ b/actionpack/test/controller/content_type_test.rb
@@ -145,18 +145,6 @@ end
 class AcceptBasedContentTypeTest < ActionController::TestCase
   tests OldContentTypeController
 
-  def setup
-    super
-    @_old_accept_header = ActionController::Base.use_accept_header
-    ActionController::Base.use_accept_header = true
-  end
-
-  def teardown
-    super
-    ActionController::Base.use_accept_header = @_old_accept_header
-  end
-
-
   def test_render_default_content_types_for_respond_to
     @request.accept = Mime::HTML.to_s
     get :render_default_content_types_for_respond_to
diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb
index 5e34773899..5c1eaf453c 100644
--- a/actionpack/test/controller/mime_responds_test.rb
+++ b/actionpack/test/controller/mime_responds_test.rb
@@ -155,13 +155,11 @@ class RespondToControllerTest < ActionController::TestCase
 
   def setup
     super
-    ActionController::Base.use_accept_header = true
     @request.host = "www.example.com"
   end
 
   def teardown
     super
-    ActionController::Base.use_accept_header = false
   end
 
   def test_html
@@ -544,13 +542,11 @@ class RespondWithControllerTest < ActionController::TestCase
 
   def setup
     super
-    ActionController::Base.use_accept_header = true
     @request.host = "www.example.com"
   end
 
   def teardown
     super
-    ActionController::Base.use_accept_header = false
   end
 
   def test_using_resource
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 84c06ca113..78200ca17e 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -435,11 +435,4 @@ protected
   def with_set(*args)
     args
   end
-
-  def with_accept_header(value)
-    ActionController::Base.use_accept_header, old = value, ActionController::Base.use_accept_header
-    yield
-  ensure
-    ActionController::Base.use_accept_header = old
-  end
 end

From 902d5a4f05c879674a3d010ac8ca76902308e18e Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 09:41:23 -0800
Subject: [PATCH 09/19] Indicate that
 ActionController::Base.resource_action_separator is deprecated and only has
 an effect with the deprecated router DSL.

---
 .../lib/action_controller/metal/compatibility.rb      | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index d6d8d612a3..1a4d0349fe 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -32,8 +32,15 @@ module ActionController
                                              @_response)
 
       # Controls the resource action separator
-      cattr_accessor :resource_action_separator
-      self.resource_action_separator = "/"
+      def self.resource_action_separator
+        @resource_action_separator ||= "/"
+      end
+
+      def self.resource_action_separator=(val)
+        ActiveSupport::Deprecation.warn "ActionController::Base.resource_action_separator is deprecated and only " \
+                                        "works with the deprecated router DSL."
+        @resource_action_separator = val
+      end
 
       def self.use_accept_header
         ActiveSupport::Deprecation.warn "ActionController::Base.use_accept_header doesn't do anything anymore. " \

From 9a9caf646d020e33ccdeac0f9b114acec019b599 Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 11:01:49 -0800
Subject: [PATCH 10/19] Add a BlockUntrustedIps middleware

---
 actionpack/lib/action_dispatch.rb             |  1 +
 .../middleware/block_untrusted_ips.rb         | 25 +++++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb

diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb
index 479ea959e6..1abb283b11 100644
--- a/actionpack/lib/action_dispatch.rb
+++ b/actionpack/lib/action_dispatch.rb
@@ -42,6 +42,7 @@ module ActionDispatch
   end
 
   autoload_under 'middleware' do
+    autoload :BlockUntrustedIps
     autoload :Callbacks
     autoload :Cascade
     autoload :Cookies
diff --git a/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb b/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb
new file mode 100644
index 0000000000..8aed0c45a6
--- /dev/null
+++ b/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb
@@ -0,0 +1,25 @@
+module ActionDispatch
+  class BlockUntrustedIps
+    class SpoofAttackError < StandardError ; end
+
+    def initialize(app)
+      @app = app
+    end
+
+    def call(env)
+      if @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_CLIENT_IP']
+        remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',')
+
+        unless remote_ips.include?(@env['HTTP_CLIENT_IP'])
+          http_client_ip     = @env['HTTP_CLIENT_IP'].inspect
+          http_forwarded_for = @env['HTTP_X_FORWARDED_FOR'].inspect
+
+          raise SpoofAttackError, "IP spoofing attack?!\n  " \
+            "HTTP_CLIENT_IP=#{http_client_ip}\n  HTTP_X_FORWARDED_FOR=http_forwarded_for"
+        end
+      end
+
+      @app.call(env)
+    end
+  end
+end
\ No newline at end of file

From 93422af5d5bc0285bd72cfb2fd9b59f6d64ba141 Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Wed, 3 Mar 2010 16:22:30 -0800
Subject: [PATCH 11/19] Move remote_ip to a middleware:

  * ActionController::Base.ip_spoofing_check deprecated => config.action_dispatch.ip_spoofing_check
  * ActionController::Base.trusted_proxies deprecated => config.action_dispatch.trusted_proxies
---
 actionpack/lib/action_dispatch.rb             |  2 +-
 .../lib/action_dispatch/http/request.rb       | 31 +----------
 .../middleware/block_untrusted_ips.rb         | 25 ---------
 .../action_dispatch/middleware/remote_ip.rb   | 51 ++++++++++++++++++
 actionpack/lib/action_dispatch/railtie.rb     |  1 +
 actionpack/test/dispatch/request_test.rb      | 12 +++--
 railties/lib/rails/configuration.rb           |  5 +-
 railties/lib/rails/log_subscriber.rb          |  1 +
 .../middleware_stack_defaults_test.rb         | 53 +++++++++++++++++++
 9 files changed, 118 insertions(+), 63 deletions(-)
 delete mode 100644 actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb
 create mode 100644 actionpack/lib/action_dispatch/middleware/remote_ip.rb
 create mode 100644 railties/test/application/middleware_stack_defaults_test.rb

diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb
index 1abb283b11..dfb8919561 100644
--- a/actionpack/lib/action_dispatch.rb
+++ b/actionpack/lib/action_dispatch.rb
@@ -42,13 +42,13 @@ module ActionDispatch
   end
 
   autoload_under 'middleware' do
-    autoload :BlockUntrustedIps
     autoload :Callbacks
     autoload :Cascade
     autoload :Cookies
     autoload :Flash
     autoload :Head
     autoload :ParamsParser
+    autoload :RemoteIp
     autoload :Rescue
     autoload :ShowExceptions
     autoload :Static
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 7a17023ed2..56a2b9bf6a 100755
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -119,36 +119,7 @@ module ActionDispatch
     # delimited list in the case of multiple chained proxies; the last
     # address which is not trusted is the originating IP.
     def remote_ip
-      remote_addr_list = @env['REMOTE_ADDR'] && @env['REMOTE_ADDR'].scan(/[^,\s]+/)
-
-      unless remote_addr_list.blank?
-        not_trusted_addrs = remote_addr_list.reject {|addr| addr =~ TRUSTED_PROXIES || addr =~ ActionController::Base.trusted_proxies}
-        return not_trusted_addrs.first unless not_trusted_addrs.empty?
-      end
-      remote_ips = @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_X_FORWARDED_FOR'].split(',')
-
-      if @env.include? 'HTTP_CLIENT_IP'
-        if ActionController::Base.ip_spoofing_check && remote_ips && !remote_ips.include?(@env['HTTP_CLIENT_IP'])
-          # We don't know which came from the proxy, and which from the user
-          raise ActionController::ActionControllerError.new <<EOM
-IP spoofing attack?!
-HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}
-HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}
-EOM
-        end
-
-        return @env['HTTP_CLIENT_IP']
-      end
-
-      if remote_ips
-        while remote_ips.size > 1 && (TRUSTED_PROXIES =~ remote_ips.last.strip || ActionController::Base.trusted_proxies =~ remote_ips.last.strip)
-          remote_ips.pop
-        end
-
-        return remote_ips.last.strip
-      end
-
-      @env['REMOTE_ADDR']
+      (@env["action_dispatch.remote_ip"] || ip).to_s
     end
 
     # Returns the lowercase name of the HTTP server software.
diff --git a/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb b/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb
deleted file mode 100644
index 8aed0c45a6..0000000000
--- a/actionpack/lib/action_dispatch/middleware/block_untrusted_ips.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-module ActionDispatch
-  class BlockUntrustedIps
-    class SpoofAttackError < StandardError ; end
-
-    def initialize(app)
-      @app = app
-    end
-
-    def call(env)
-      if @env['HTTP_X_FORWARDED_FOR'] && @env['HTTP_CLIENT_IP']
-        remote_ips = @env['HTTP_X_FORWARDED_FOR'].split(',')
-
-        unless remote_ips.include?(@env['HTTP_CLIENT_IP'])
-          http_client_ip     = @env['HTTP_CLIENT_IP'].inspect
-          http_forwarded_for = @env['HTTP_X_FORWARDED_FOR'].inspect
-
-          raise SpoofAttackError, "IP spoofing attack?!\n  " \
-            "HTTP_CLIENT_IP=#{http_client_ip}\n  HTTP_X_FORWARDED_FOR=http_forwarded_for"
-        end
-      end
-
-      @app.call(env)
-    end
-  end
-end
\ No newline at end of file
diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
new file mode 100644
index 0000000000..c7d710b98e
--- /dev/null
+++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
@@ -0,0 +1,51 @@
+module ActionDispatch
+  class RemoteIp
+    class IpSpoofAttackError < StandardError ; end
+
+    class RemoteIpGetter
+      def initialize(env, check_ip_spoofing, trusted_proxies)
+        @env = env
+        @check_ip_spoofing = check_ip_spoofing
+        @trusted_proxies = trusted_proxies
+      end
+
+      def remote_addrs
+        @remote_addrs ||= begin
+          list = @env['REMOTE_ADDR'] ? @env['REMOTE_ADDR'].split(/[,\s]+/) : []
+          list.reject { |addr| addr =~ @trusted_proxies }
+        end
+      end
+
+      def to_s
+        return remote_addrs.first if remote_addrs.any?
+
+        forwarded_ips = @env['HTTP_X_FORWARDED_FOR'] ? @env['HTTP_X_FORWARDED_FOR'].strip.split(/[,\s]+/) : []
+
+        if client_ip = @env['HTTP_CLIENT_IP']
+          if @check_ip_spoofing && !forwarded_ips.include?(client_ip)
+            # We don't know which came from the proxy, and which from the user
+            raise IpSpoofAttackError, "IP spoofing attack?!" \
+              "HTTP_CLIENT_IP=#{@env['HTTP_CLIENT_IP'].inspect}" \
+              "HTTP_X_FORWARDED_FOR=#{@env['HTTP_X_FORWARDED_FOR'].inspect}"
+          end
+          return client_ip
+        end
+
+        return forwarded_ips.reject { |ip| ip =~ @trusted_proxies }.last || @env["REMOTE_ADDR"]
+      end
+    end
+
+    def initialize(app, check_ip_spoofing = true, trusted_proxies = nil)
+      @app = app
+      @check_ip_spoofing = check_ip_spoofing
+      regex = '(^127\.0\.0\.1$|^(10|172\.(1[6-9]|2[0-9]|30|31)|192\.168)\.)'
+      regex << "|(#{trusted_proxies})" if trusted_proxies
+      @trusted_proxies = Regexp.new(regex, "i")
+    end
+
+    def call(env)
+      env["action_dispatch.remote_ip"] = RemoteIpGetter.new(env, @check_ip_spoofing, @trusted_proxies)
+      @app.call(env)
+    end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb
index 79e9464337..e486bd4079 100644
--- a/actionpack/lib/action_dispatch/railtie.rb
+++ b/actionpack/lib/action_dispatch/railtie.rb
@@ -6,6 +6,7 @@ module ActionDispatch
     railtie_name :action_dispatch
 
     config.action_dispatch.x_sendfile_header = "X-Sendfile"
+    config.action_dispatch.ip_spoofing_check = true
 
     # Prepare dispatcher callbacks and run 'prepare' callbacks
     initializer "action_dispatch.prepare_dispatcher" do |app|
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 78200ca17e..3a07185586 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -42,7 +42,7 @@ class RequestTest < ActiveSupport::TestCase
 
     request = stub_request 'HTTP_X_FORWARDED_FOR' => '1.1.1.1',
                            'HTTP_CLIENT_IP'       => '2.2.2.2'
-    e = assert_raise(ActionController::ActionControllerError) {
+    e = assert_raise(ActionDispatch::RemoteIp::IpSpoofAttackError) {
       request.remote_ip
     }
     assert_match /IP spoofing attack/, e.message
@@ -54,18 +54,17 @@ class RequestTest < ActiveSupport::TestCase
     # example is WAP.  Since the cellular network is not IP based, it's a
     # leap of faith to assume that their proxies are ever going to set the
     # HTTP_CLIENT_IP/HTTP_X_FORWARDED_FOR headers properly.
-    ActionController::Base.ip_spoofing_check = false
     request = stub_request 'HTTP_X_FORWARDED_FOR' => '1.1.1.1',
-                           'HTTP_CLIENT_IP'       => '2.2.2.2'
+                           'HTTP_CLIENT_IP'       => '2.2.2.2',
+                           :ip_spoofing_check => false
     assert_equal '2.2.2.2', request.remote_ip
-    ActionController::Base.ip_spoofing_check = true
 
     request = stub_request 'HTTP_X_FORWARDED_FOR' => '8.8.8.8, 9.9.9.9'
     assert_equal '9.9.9.9', request.remote_ip
   end
 
   test "remote ip with user specified trusted proxies" do
-    ActionController::Base.trusted_proxies = /^67\.205\.106\.73$/i
+    @trusted_proxies = /^67\.205\.106\.73$/i
 
     request = stub_request 'REMOTE_ADDR' => '67.205.106.73',
                            'HTTP_X_FORWARDED_FOR' => '3.4.5.6'
@@ -429,6 +428,9 @@ class RequestTest < ActiveSupport::TestCase
 protected
 
   def stub_request(env = {})
+    ip_spoofing_check = env.key?(:ip_spoofing_check) ? env.delete(:ip_spoofing_check) : true
+    ip_app = ActionDispatch::RemoteIp.new(Proc.new { }, ip_spoofing_check, @trusted_proxies)
+    ip_app.call(env)
     ActionDispatch::Request.new(env)
   end
 
diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb
index a1e901f04f..a26978ef92 100644
--- a/railties/lib/rails/configuration.rb
+++ b/railties/lib/rails/configuration.rb
@@ -84,11 +84,12 @@ module Rails
           middleware.use('::Rack::Runtime')
           middleware.use('::Rails::Rack::Logger')
           middleware.use('::ActionDispatch::ShowExceptions', lambda { consider_all_requests_local })
+          middleware.use("::ActionDispatch::RemoteIp", lambda { action_dispatch.ip_spoofing_check }, lambda { action_dispatch.trusted_proxies })
           middleware.use('::Rack::Sendfile', lambda { action_dispatch.x_sendfile_header })
           middleware.use('::ActionDispatch::Callbacks', lambda { !cache_classes })
           middleware.use('::ActionDispatch::Cookies')
-          middleware.use(lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options })
-          middleware.use('::ActionDispatch::Flash', :if => lambda { ActionController::Base.session_store })
+          middleware.use(lambda { action_controller.session_store }, lambda { action_controller.session_options })
+          middleware.use('::ActionDispatch::Flash', :if => lambda { action_controller.session_store })
           middleware.use(lambda { metal_loader.build_middleware(metals) }, :if => lambda { metal_loader.metals.any? })
           middleware.use('ActionDispatch::ParamsParser')
           middleware.use('::Rack::MethodOverride')
diff --git a/railties/lib/rails/log_subscriber.rb b/railties/lib/rails/log_subscriber.rb
index 0fbc19d89c..42697d2e32 100644
--- a/railties/lib/rails/log_subscriber.rb
+++ b/railties/lib/rails/log_subscriber.rb
@@ -87,6 +87,7 @@ module Rails
     %w(info debug warn error fatal unknown).each do |level|
       class_eval <<-METHOD, __FILE__, __LINE__ + 1
         def #{level}(*args, &block)
+          return unless logger
           logger.#{level}(*args, &block)
         end
       METHOD
diff --git a/railties/test/application/middleware_stack_defaults_test.rb b/railties/test/application/middleware_stack_defaults_test.rb
new file mode 100644
index 0000000000..94151a90da
--- /dev/null
+++ b/railties/test/application/middleware_stack_defaults_test.rb
@@ -0,0 +1,53 @@
+require 'isolation/abstract_unit'
+
+class MiddlewareStackDefaultsTest < Test::Unit::TestCase
+  include ActiveSupport::Testing::Isolation
+
+  def setup
+    boot_rails
+    require "rails"
+    require "action_controller/railtie"
+
+    Object.const_set(:MyApplication, Class.new(Rails::Application))
+    MyApplication.class_eval do
+      config.action_controller.session = { :key => "_myapp_session", :secret => "OMG A SEKRET" * 10 }
+    end
+  end
+
+  def remote_ip(env = {})
+    remote_ip = nil
+    env = Rack::MockRequest.env_for("/").merge(env).merge('action_dispatch.show_exceptions' => false)
+
+    endpoint = Proc.new do |e|
+      remote_ip = ActionDispatch::Request.new(e).remote_ip
+      [200, {}, ["Hello"]]
+    end
+
+    out = MyApplication.middleware.build(endpoint).call(env)
+    remote_ip
+  end
+
+  test "remote_ip works" do
+    assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "1.1.1.1")
+  end
+
+  test "checks IP spoofing by default" do
+    assert_raises(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+      remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2")
+    end
+  end
+
+  test "can disable IP spoofing check" do
+    MyApplication.config.action_dispatch.ip_spoofing_check = false
+
+    assert_nothing_raised(ActionDispatch::RemoteIp::IpSpoofAttackError) do
+      assert_equal "1.1.1.2", remote_ip("HTTP_X_FORWARDED_FOR" => "1.1.1.1", "HTTP_CLIENT_IP" => "1.1.1.2")
+    end
+  end
+
+  test "the user can set trusted proxies" do
+    MyApplication.config.action_dispatch.trusted_proxies = /^4\.2\.42\.42$/
+
+    assert_equal "1.1.1.1", remote_ip("REMOTE_ADDR" => "4.2.42.42,1.1.1.1")
+  end
+end

From 52efbdcdefa678ae2ee9530bcfefd0e2529b188f Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Wed, 3 Mar 2010 16:23:00 -0800
Subject: [PATCH 12/19] Add caller to request_uri deprecation notice

---
 actionpack/lib/action_dispatch/http/url.rb | 2 +-
 railties/lib/rails/rack/logger.rb          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index f7afbe7fed..b64a83c62e 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -88,7 +88,7 @@ module ActionDispatch
       # Returns the request URI, accounting for server idiosyncrasies.
       # WEBrick includes the full URL. IIS leaves REQUEST_URI blank.
       def request_uri
-        ActiveSupport::Deprecation.warn "Using #request_uri is deprecated. Use fullpath instead."
+        ActiveSupport::Deprecation.warn "Using #request_uri is deprecated. Use fullpath instead.", caller
         fullpath
       end
 
diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb
index 2efe224e94..dd8b342f59 100644
--- a/railties/lib/rails/rack/logger.rb
+++ b/railties/lib/rails/rack/logger.rb
@@ -19,7 +19,7 @@ module Rails
 
         def before_dispatch(env)
           request = ActionDispatch::Request.new(env)
-          path = request.request_uri.inspect rescue "unknown"
+          path = request.fullpath.inspect rescue "unknown"
 
           info "\n\nStarted #{request.method.to_s.upcase} #{path} " <<
                       "for #{request.remote_ip} at #{Time.now.to_s(:db)}"

From 786724107c3dedaeccaae9e187458b48df472f90 Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Wed, 3 Mar 2010 16:32:27 -0800
Subject: [PATCH 13/19] Deprecate IP spoofing settings that are directly on the
 controller in favor of configuring a middleware

---
 .../action_controller/metal/compatibility.rb  | 33 +++++++++++++++----
 actionpack/test/dispatch/request_test.rb      |  2 --
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 1a4d0349fe..ab59ffc0b4 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -57,11 +57,6 @@ module ActionController
       # and images to a dedicated asset server away from the main web server. Example:
       #   ActionController::Base.asset_host = "http://assets.example.com"
       cattr_accessor :asset_host
-
-      cattr_accessor :ip_spoofing_check
-      self.ip_spoofing_check = true
-
-      cattr_accessor :trusted_proxies
     end
 
     def self.deprecated_config_accessor(option, message = nil)
@@ -119,7 +114,7 @@ module ActionController
       end
 
       def consider_all_requests_local=(value)
-        ActiveSupport::Deprecation.warn "ActionController::Base.consider_all_requests_local= is no longer effective. " <<
+        ActiveSupport::Deprecation.warn "ActionController::Base.consider_all_requests_local= is deprecated. " <<
           "Please configure it on your application with config.consider_all_requests_local="
         Rails.application.config.consider_all_requests_local = value
       end
@@ -131,11 +126,35 @@ module ActionController
       end
 
       def allow_concurrency=(value)
-        ActiveSupport::Deprecation.warn "ActionController::Base.allow_concurrency= is no longer effective. " <<
+        ActiveSupport::Deprecation.warn "ActionController::Base.allow_concurrency= is deprecated. " <<
           "Please configure it on your application with config.allow_concurrency="
         Rails.application.config.allow_concurrency = value
       end
 
+      def ip_spoofing_check=(value)
+        ActiveSupport::Deprecation.warn "ActionController::Base.ip_spoofing_check= is deprecated. " <<
+          "Please configure it on your application with config.action_dispatch.ip_spoofing_check="
+        Rails.application.config.action_disaptch.ip_spoofing_check = value
+      end
+
+      def ip_spoofing_check
+        ActiveSupport::Deprecation.warn "ActionController::Base.ip_spoofing_check is deprecated. "
+          "Configuring ip_spoofing_check on the application configures a middleware."
+        Rails.application.config.action_disaptch.ip_spoofing_check
+      end
+
+      def trusted_proxies=(value)
+        ActiveSupport::Deprecation.warn "ActionController::Base.trusted_proxies= is deprecated. " <<
+          "Please configure it on your application with config.action_dispatch.trusted_proxies="
+        Rails.application.config.action_dispatch.ip_spoofing_check = value
+      end
+
+      def trusted_proxies
+        ActiveSupport::Deprecation.warn "ActionController::Base.trusted_proxies is deprecated. " <<
+          "Configuring trusted_proxies on the application configures a middleware."
+        Rails.application.config.action_dispatch.ip_spoofing_check = value
+      end
+
       def rescue_action(env)
         raise env["action_dispatch.rescue.exception"]
       end
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 3a07185586..badef4e92e 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -87,8 +87,6 @@ class RequestTest < ActiveSupport::TestCase
 
     request = stub_request 'HTTP_X_FORWARDED_FOR' => '9.9.9.9, 3.4.5.6, 10.0.0.1, 67.205.106.73'
     assert_equal '3.4.5.6', request.remote_ip
-
-    ActionController::Base.trusted_proxies = nil
   end
 
   test "domains" do

From 54302ef55bffd1a93f20f5d1ae81217b2e4149c4 Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Wed, 3 Mar 2010 17:08:54 -0800
Subject: [PATCH 14/19] Add caller to deprecation notices

---
 .../action_controller/metal/compatibility.rb  | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index ab59ffc0b4..9471f36de3 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -70,7 +70,7 @@ module ActionController
 
       ClassMethods.class_eval <<-RUBY, __FILE__, __LINE__ + 1
         def #{option}
-          ActiveSupport::Deprecation.warn #{message.inspect}
+          ActiveSupport::Deprecation.warn #{message.inspect}, caller
           config.#{option}
         end
       RUBY
@@ -82,7 +82,7 @@ module ActionController
 
       ClassMethods.class_eval <<-RUBY, __FILE__, __LINE__ + 1
         def #{option}=(val)
-          ActiveSupport::Deprecation.warn #{message.inspect}
+          ActiveSupport::Deprecation.warn #{message.inspect}, caller
           config.#{option} = val
         end
       RUBY
@@ -109,49 +109,49 @@ module ActionController
     module ClassMethods
       def consider_all_requests_local
         ActiveSupport::Deprecation.warn "ActionController::Base.consider_all_requests_local is deprecated, " <<
-          "use Rails.application.config.consider_all_requests_local instead"
+          "use Rails.application.config.consider_all_requests_local instead", caller
         Rails.application.config.consider_all_requests_local
       end
 
       def consider_all_requests_local=(value)
         ActiveSupport::Deprecation.warn "ActionController::Base.consider_all_requests_local= is deprecated. " <<
-          "Please configure it on your application with config.consider_all_requests_local="
+          "Please configure it on your application with config.consider_all_requests_local=", caller
         Rails.application.config.consider_all_requests_local = value
       end
 
       def allow_concurrency
         ActiveSupport::Deprecation.warn "ActionController::Base.allow_concurrency is deprecated, " <<
-          "use Rails.application.config.allow_concurrency instead"
+          "use Rails.application.config.allow_concurrency instead", caller
         Rails.application.config.allow_concurrency
       end
 
       def allow_concurrency=(value)
         ActiveSupport::Deprecation.warn "ActionController::Base.allow_concurrency= is deprecated. " <<
-          "Please configure it on your application with config.allow_concurrency="
+          "Please configure it on your application with config.allow_concurrency=", caller
         Rails.application.config.allow_concurrency = value
       end
 
       def ip_spoofing_check=(value)
         ActiveSupport::Deprecation.warn "ActionController::Base.ip_spoofing_check= is deprecated. " <<
-          "Please configure it on your application with config.action_dispatch.ip_spoofing_check="
+          "Please configure it on your application with config.action_dispatch.ip_spoofing_check=", caller
         Rails.application.config.action_disaptch.ip_spoofing_check = value
       end
 
       def ip_spoofing_check
-        ActiveSupport::Deprecation.warn "ActionController::Base.ip_spoofing_check is deprecated. "
-          "Configuring ip_spoofing_check on the application configures a middleware."
+        ActiveSupport::Deprecation.warn "ActionController::Base.ip_spoofing_check is deprecated. " <<
+          "Configuring ip_spoofing_check on the application configures a middleware.", caller
         Rails.application.config.action_disaptch.ip_spoofing_check
       end
 
       def trusted_proxies=(value)
         ActiveSupport::Deprecation.warn "ActionController::Base.trusted_proxies= is deprecated. " <<
-          "Please configure it on your application with config.action_dispatch.trusted_proxies="
+          "Please configure it on your application with config.action_dispatch.trusted_proxies=", caller
         Rails.application.config.action_dispatch.ip_spoofing_check = value
       end
 
       def trusted_proxies
         ActiveSupport::Deprecation.warn "ActionController::Base.trusted_proxies is deprecated. " <<
-          "Configuring trusted_proxies on the application configures a middleware."
+          "Configuring trusted_proxies on the application configures a middleware.", caller
         Rails.application.config.action_dispatch.ip_spoofing_check = value
       end
 

From b160663bd13d08bf845bc8cdf87a2c5e7e46f901 Mon Sep 17 00:00:00 2001
From: Carlhuda <carlhuda@engineyard.com>
Date: Wed, 3 Mar 2010 17:36:08 -0800
Subject: [PATCH 15/19] Start refactoring the method of configuring ActionView

---
 actionpack/lib/action_controller/railtie.rb       | 14 ++++++++++++++
 actionpack/lib/action_view/base.rb                | 12 ++----------
 .../lib/action_view/helpers/asset_tag_helper.rb   |  7 -------
 actionpack/lib/action_view/test_case.rb           |  4 ++++
 actionpack/test/abstract_unit.rb                  | 15 +++++++++++++++
 actionpack/test/template/asset_tag_helper_test.rb | 14 ++------------
 actionpack/test/template/form_tag_helper_test.rb  |  9 ++++-----
 actionpack/test/template/url_helper_test.rb       |  9 ++-------
 railties/lib/rails/engine/configuration.rb        |  3 +++
 9 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index 07c6b8f2b6..f4c51ba760 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -17,7 +17,21 @@ module ActionController
       ActionController::Base.logger ||= Rails.logger
     end
 
+    # assets_dir = defined?(Rails.public_path) ? Rails.public_path : "public"
+    # ActionView::DEFAULT_CONFIG = {
+    #   :assets_dir => assets_dir,
+    #   :javascripts_dir => "#{assets_dir}/javascripts",
+    #   :stylesheets_dir => "#{assets_dir}/stylesheets",
+    # }
+
+
     initializer "action_controller.set_configs" do |app|
+      paths = app.config.paths
+      ac = app.config.action_controller
+      ac.assets_dir = paths.public
+      ac.javascripts_dir = paths.public.javascripts
+      ac.stylesheets_dir = paths.public.stylesheets
+
       app.config.action_controller.each do |k,v|
         ActionController::Base.send "#{k}=", v
       end
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 2d2b53a6ce..87a1972fb4 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -175,15 +175,6 @@ module ActionView #:nodoc:
 
     include Helpers, Rendering, Partials, ::ERB::Util
 
-    def config
-      self.config = DEFAULT_CONFIG unless @config
-      @config
-    end
-
-    def config=(config)
-      @config = ActiveSupport::OrderedOptions.new.merge(config)
-    end
-
     extend ActiveSupport::Memoizable
 
     attr_accessor :base_path, :assigns, :template_extension
@@ -306,12 +297,13 @@ module ActionView #:nodoc:
       @helpers = self.class.helpers || Module.new
 
       @_controller   = controller
+      @_config       = controller.config if controller
       @_content_for  = Hash.new {|h,k| h[k] = ActiveSupport::SafeBuffer.new }
       @_virtual_path = nil
       self.view_paths = view_paths
     end
 
-    attr_internal :controller, :template
+    attr_internal :controller, :template, :config
     attr_reader :view_paths
 
     def view_paths=(paths)
diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
index 5f76ff456e..b52d255d1f 100644
--- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
@@ -133,13 +133,6 @@ module ActionView
     # change. You can use something like Live HTTP Headers for Firefox to verify
     # that the cache is indeed working.
     module AssetTagHelper
-      assets_dir = defined?(Rails.public_path) ? Rails.public_path : "public"
-      ActionView::DEFAULT_CONFIG = {
-        :assets_dir => assets_dir,
-        :javascripts_dir => "#{assets_dir}/javascripts",
-        :stylesheets_dir => "#{assets_dir}/stylesheets",
-      }
-
       JAVASCRIPT_DEFAULT_SOURCES = ['prototype', 'effects', 'dragdrop', 'controls', 'rails'].freeze unless const_defined?(:JAVASCRIPT_DEFAULT_SOURCES)
 
       # Returns a link tag that browsers and news readers can use to auto-detect
diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb
index 12142a3728..72dbead14b 100644
--- a/actionpack/lib/action_view/test_case.rb
+++ b/actionpack/lib/action_view/test_case.rb
@@ -62,6 +62,10 @@ module ActionView
       make_test_case_available_to_view!
     end
 
+    def config
+      @controller.config
+    end
+
     def render(options = {}, local_assigns = {}, &block)
       @rendered << output = _view.render(options, local_assigns, &block)
       output
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index f1e8779cfa..7223a7824e 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -105,6 +105,21 @@ class RoutedRackApp
   end
 end
 
+class BasicController
+  attr_accessor :request
+
+  def config
+    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Metal.config).tap do |config|
+      # VIEW TODO: View tests should not require a controller
+      public_dir = File.expand_path("../fixtures/public", __FILE__)
+      config.assets_dir = public_dir
+      config.javascripts_dir = "#{public_dir}/javascripts"
+      config.stylesheets_dir = "#{public_dir}/stylesheets"
+      config
+    end
+  end
+end
+
 class ActionController::IntegrationTest < ActiveSupport::TestCase
   def self.build_app(routes = nil)
     RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware|
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index ccc39e9af6..8b24f66a6e 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -12,13 +12,6 @@ end
 class AssetTagHelperTest < ActionView::TestCase
   tests ActionView::Helpers::AssetTagHelper
 
-  DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG.merge(
-    :assets_dir => File.dirname(__FILE__) + "/../fixtures/public",
-    :javascripts_dir => File.dirname(__FILE__) + "/../fixtures/public/javascripts",
-    :stylesheets_dir => File.dirname(__FILE__) + "/../fixtures/public/stylesheets")
-
-  include ActiveSupport::Configurable
-
   def setup
     super
     silence_warnings do
@@ -41,7 +34,7 @@ class AssetTagHelperTest < ActionView::TestCase
       )
     end
 
-    @controller = Class.new(FakeController) do
+    @controller = Class.new(BasicController) do
       def url_for(*args) "http://www.example.com" end
     end.new
 
@@ -883,12 +876,9 @@ end
 class AssetTagHelperNonVhostTest < ActionView::TestCase
   tests ActionView::Helpers::AssetTagHelper
 
-  DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG
-  include ActiveSupport::Configurable
-
   def setup
     super
-    @controller = Class.new(FakeController) do
+    @controller = Class.new(BasicController) do
       def url_for(options)
         "http://www.example.com/collaboration/hieraki"
       end
diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb
index 0ceec1afbf..c7d4bc986c 100644
--- a/actionpack/test/template/form_tag_helper_test.rb
+++ b/actionpack/test/template/form_tag_helper_test.rb
@@ -3,17 +3,16 @@ require 'abstract_unit'
 class FormTagHelperTest < ActionView::TestCase
   tests ActionView::Helpers::FormTagHelper
 
-  include ActiveSupport::Configurable
-  DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG
+  # include ActiveSupport::Configurable
+  # DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG
 
   def setup
     super
-    @controller = Class.new do
+    @controller = Class.new(BasicController) do
       def url_for(options)
         "http://www.example.com"
       end
-    end
-    @controller = @controller.new
+    end.new
   end
 
   VALID_HTML_ID = /^[A-Za-z][-_:.A-Za-z0-9]*$/ # see http://www.w3.org/TR/html4/types.html#type-name
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index d8ccb380a6..533a07b6f4 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -4,19 +4,14 @@ require 'active_support/ordered_options'
 require 'controller/fake_controllers'
 
 class UrlHelperTest < ActionView::TestCase
-  include ActiveSupport::Configurable
-  DEFAULT_CONFIG = ActionView::DEFAULT_CONFIG
 
   def setup
     super
-    @controller = Class.new do
-      attr_accessor :url, :request
+    @controller = Class.new(BasicController) do
+      attr_accessor :url
       def url_for(options)
         url
       end
-      def config
-        ActiveSupport::InheritableOptions.new({})
-      end
     end
 
     @controller = @controller.new
diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb
index 93b882f874..5d3e768cfd 100644
--- a/railties/lib/rails/engine/configuration.rb
+++ b/railties/lib/rails/engine/configuration.rb
@@ -26,6 +26,9 @@ module Rails
           paths.config.initializers "config/initializers", :glob => "**/*.rb"
           paths.config.locales      "config/locales",      :glob => "*.{rb,yml}"
           paths.config.routes       "config/routes.rb"
+          paths.public              "public"
+          paths.public.javascripts  "public/javascripts"
+          paths.public.stylesheets  "public/stylesheets"
           paths
         end
       end

From 1f0f05b10c924d2f0d0ff4c74cbd979e77deea1d Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 19:45:08 -0800
Subject: [PATCH 16/19] Move the original config method onto AbstractController

---
 actionpack/lib/abstract_controller/base.rb        | 8 ++++++++
 actionpack/lib/action_controller/metal.rb         | 8 --------
 actionpack/test/abstract_unit.rb                  | 2 +-
 actionpack/test/template/asset_tag_helper_test.rb | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb
index e14818e464..cbd021e246 100644
--- a/actionpack/lib/abstract_controller/base.rb
+++ b/actionpack/lib/abstract_controller/base.rb
@@ -28,6 +28,10 @@ module AbstractController
         @descendants ||= []
       end
 
+      def config
+        @config ||= ActiveSupport::InheritableOptions.new(superclass < Base ? superclass.config : {})
+      end
+
       # A list of all internal methods for a controller. This finds the first
       # abstract superclass of a controller, and gets a list of all public
       # instance methods on that abstract class. Public instance methods of
@@ -95,6 +99,10 @@ module AbstractController
       @_formats = nil
     end
 
+    def config
+      self.class.config
+    end
+
     # Calls the action going through the entire action dispatch stack.
     #
     # The actual method that is called is determined by calling
diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb
index 1f6bbeb7c5..4374ab0a6b 100644
--- a/actionpack/lib/action_controller/metal.rb
+++ b/actionpack/lib/action_controller/metal.rb
@@ -11,14 +11,6 @@ module ActionController
   class Metal < AbstractController::Base
     abstract!
 
-    def self.config
-      @config ||= ActiveSupport::InheritableOptions.new(superclass < Metal ? superclass.config : {})
-    end
-
-    def config
-      self.class.config
-    end
-
     # :api: public
     attr_internal :params, :env
 
diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb
index 7223a7824e..29270ed228 100644
--- a/actionpack/test/abstract_unit.rb
+++ b/actionpack/test/abstract_unit.rb
@@ -109,7 +109,7 @@ class BasicController
   attr_accessor :request
 
   def config
-    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Metal.config).tap do |config|
+    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Base.config).tap do |config|
       # VIEW TODO: View tests should not require a controller
       public_dir = File.expand_path("../fixtures/public", __FILE__)
       config.assets_dir = public_dir
diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb
index 8b24f66a6e..bbfab05a0f 100644
--- a/actionpack/test/template/asset_tag_helper_test.rb
+++ b/actionpack/test/template/asset_tag_helper_test.rb
@@ -5,7 +5,7 @@ class FakeController
   attr_accessor :request
 
   def config
-    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Metal.config)
+    @config ||= ActiveSupport::InheritableOptions.new(ActionController::Base.config)
   end
 end
 

From 15b3b74624eb4c5ae383956950cab12ca9899131 Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 21:16:35 -0800
Subject: [PATCH 17/19] Fix all the broken tests due to the AC configuration
 refactor

---
 .../action_controller/metal/compatibility.rb  |  3 +++
 .../metal/session_management.rb               | 22 +++++++++++--------
 actionpack/lib/action_controller/railtie.rb   |  3 +++
 railties/lib/rails/configuration.rb           |  2 +-
 railties/test/application/metal_test.rb       |  4 ++--
 railties/test/application/middleware_test.rb  |  1 +
 railties/test/railties/shared_tests.rb        |  2 +-
 7 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb
index 9471f36de3..4c2136de8a 100644
--- a/actionpack/lib/action_controller/metal/compatibility.rb
+++ b/actionpack/lib/action_controller/metal/compatibility.rb
@@ -91,6 +91,9 @@ module ActionController
     deprecated_config_writer :session_store
     deprecated_config_writer :session_options
     deprecated_config_accessor :relative_url_root, "relative_url_root is ineffective. Please stop using it"
+    deprecated_config_accessor :assets_dir
+    deprecated_config_accessor :javascripts_dir
+    deprecated_config_accessor :stylesheets_dir
 
     # For old tests
     def initialize_template_class(*) end
diff --git a/actionpack/lib/action_controller/metal/session_management.rb b/actionpack/lib/action_controller/metal/session_management.rb
index 09ef9261a4..ce8b20964b 100644
--- a/actionpack/lib/action_controller/metal/session_management.rb
+++ b/actionpack/lib/action_controller/metal/session_management.rb
@@ -3,24 +3,28 @@ module ActionController #:nodoc:
     extend ActiveSupport::Concern
 
     included do
-      self.config.session_store   ||= :cookie_store
+      # This is still needed for the session secret for some reason.
       self.config.session_options ||= {}
     end
 
+    def self.session_store_for(store)
+      case store
+      when :active_record_store
+        ActiveRecord::SessionStore
+      when Symbol
+        ActionDispatch::Session.const_get(store.to_s.camelize)
+      else
+        store
+      end
+    end
+
     module ClassMethods
       def session_options
         config.session_options
       end
 
       def session_store
-        case store = config.session_store
-        when :active_record_store
-          ActiveRecord::SessionStore
-        when Symbol
-          ActionDispatch::Session.const_get(store.to_s.camelize)
-        else
-          store
-        end
+        SessionManagement.session_store_for(config.session_store)
       end
 
       def session=(options = {})
diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb
index f4c51ba760..3a084cdf67 100644
--- a/actionpack/lib/action_controller/railtie.rb
+++ b/actionpack/lib/action_controller/railtie.rb
@@ -13,6 +13,9 @@ module ActionController
 
     log_subscriber ActionController::Railties::LogSubscriber.new
 
+    config.action_controller.session_store = :cookie_store
+    config.action_controller.session_options = {}
+
     initializer "action_controller.logger" do
       ActionController::Base.logger ||= Rails.logger
     end
diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb
index a26978ef92..a78e115fe1 100644
--- a/railties/lib/rails/configuration.rb
+++ b/railties/lib/rails/configuration.rb
@@ -88,7 +88,7 @@ module Rails
           middleware.use('::Rack::Sendfile', lambda { action_dispatch.x_sendfile_header })
           middleware.use('::ActionDispatch::Callbacks', lambda { !cache_classes })
           middleware.use('::ActionDispatch::Cookies')
-          middleware.use(lambda { action_controller.session_store }, lambda { action_controller.session_options })
+          middleware.use(lambda { ActionController::SessionManagement.session_store_for(action_controller.session_store) }, lambda { action_controller.session })
           middleware.use('::ActionDispatch::Flash', :if => lambda { action_controller.session_store })
           middleware.use(lambda { metal_loader.build_middleware(metals) }, :if => lambda { metal_loader.metals.any? })
           middleware.use('ActionDispatch::ParamsParser')
diff --git a/railties/test/application/metal_test.rb b/railties/test/application/metal_test.rb
index 225bede117..1ec62282c8 100644
--- a/railties/test/application/metal_test.rb
+++ b/railties/test/application/metal_test.rb
@@ -28,7 +28,7 @@ module ApplicationTests
         end
       RUBY
 
-      get "/"
+      get "/not/slash"
       assert_equal 200, last_response.status
       assert_equal "FooMetal", last_response.body
     end
@@ -50,7 +50,7 @@ module ApplicationTests
         end
       RUBY
 
-      get "/"
+      get "/not/slash"
       assert_equal 200, last_response.status
       assert_equal "Metal B", last_response.body
     end
diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb
index 5e869bff1e..9a359d20b1 100644
--- a/railties/test/application/middleware_test.rb
+++ b/railties/test/application/middleware_test.rb
@@ -19,6 +19,7 @@ module ApplicationTests
         "Rack::Runtime",
         "Rails::Rack::Logger",
         "ActionDispatch::ShowExceptions",
+        "ActionDispatch::RemoteIp",
         "Rack::Sendfile",
         "ActionDispatch::Callbacks",
         "ActionDispatch::Cookies",
diff --git a/railties/test/railties/shared_tests.rb b/railties/test/railties/shared_tests.rb
index 151abe21f8..0ebc8a2d3f 100644
--- a/railties/test/railties/shared_tests.rb
+++ b/railties/test/railties/shared_tests.rb
@@ -254,7 +254,7 @@ YAML
       require 'rack/test'
       extend Rack::Test::Methods
 
-      get "/"
+      get "/not/slash"
       assert_equal 200, last_response.status
       assert_equal "FooMetal", last_response.body
     end

From 7dbf5c820b3454da8194e2c823cdfd3982657f03 Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 21:29:26 -0800
Subject: [PATCH 18/19] Tweak how ActionPack handles InheritableOptions

---
 actionpack/lib/abstract_controller/base.rb | 2 ++
 actionpack/lib/action_controller/metal.rb  | 1 -
 actionpack/lib/action_view/base.rb         | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb
index cbd021e246..8e24d55ad0 100644
--- a/actionpack/lib/abstract_controller/base.rb
+++ b/actionpack/lib/abstract_controller/base.rb
@@ -1,3 +1,5 @@
+require 'active_support/ordered_options'
+
 module AbstractController
   class Error < StandardError; end
   class ActionNotFound < StandardError; end
diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb
index 4374ab0a6b..5e0ed201cb 100644
--- a/actionpack/lib/action_controller/metal.rb
+++ b/actionpack/lib/action_controller/metal.rb
@@ -1,5 +1,4 @@
 require 'active_support/core_ext/class/attribute'
-require 'active_support/ordered_options'
 
 module ActionController
   # ActionController::Metal provides a way to get a valid Rack application from a controller.
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 87a1972fb4..76f9eb2b0d 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -297,7 +297,7 @@ module ActionView #:nodoc:
       @helpers = self.class.helpers || Module.new
 
       @_controller   = controller
-      @_config       = controller.config if controller
+      @_config       = ActiveSupport::InheritableOptions.new(controller.config) if controller
       @_content_for  = Hash.new {|h,k| h[k] = ActiveSupport::SafeBuffer.new }
       @_virtual_path = nil
       self.view_paths = view_paths

From 13a932cddcd3165792d3454009b6d07471031a2c Mon Sep 17 00:00:00 2001
From: Carl Lerche <carllerche@mac.com>
Date: Wed, 3 Mar 2010 21:32:05 -0800
Subject: [PATCH 19/19] Modifying configurations on the instance of a
 controller should not affect the class

---
 actionpack/lib/abstract_controller/base.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb
index 8e24d55ad0..24f3b552ba 100644
--- a/actionpack/lib/abstract_controller/base.rb
+++ b/actionpack/lib/abstract_controller/base.rb
@@ -102,7 +102,7 @@ module AbstractController
     end
 
     def config
-      self.class.config
+      @config ||= ActiveSupport::InheritableOptions.new(self.class.config)
     end
 
     # Calls the action going through the entire action dispatch stack.