Unify ActionController::AbstractRequest and ActionController::Request

2 files changed, 81 insertions, 110 deletions

diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb
index a3e96a0fc4..71b5ebb1b3 100755
--- a/actionpack/lib/action_controller/request.rb
+++ b/actionpack/lib/action_controller/request.rb
@@ -7,9 +7,35 @@ require 'action_controller/cgi_ext'
 
 module ActionController
   # CgiRequest and TestRequest provide concrete implementations.
-  class AbstractRequest < Rack::Request
+  class Request
     extend ActiveSupport::Memoizable
 
+    class SessionFixationAttempt < StandardError #:nodoc:
+    end
+
+    attr_reader :env
+
+    def initialize(env)
+      @env = env
+    end
+
+    %w[ AUTH_TYPE GATEWAY_INTERFACE PATH_INFO
+        PATH_TRANSLATED REMOTE_HOST
+        REMOTE_IDENT REMOTE_USER SCRIPT_NAME
+        SERVER_NAME SERVER_PROTOCOL
+
+        HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING
+        HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM
+        HTTP_NEGOTIATE HTTP_PRAGMA HTTP_REFERER HTTP_USER_AGENT ].each do |env|
+      define_method(env.sub(/^HTTP_/n, '').downcase) do
+        @env[env]
+      end
+    end
+
+    def key?(key)
+      @env.key?(key)
+    end
+
     HTTP_METHODS = %w(get head put post delete options)
     HTTP_METHOD_LOOKUP = HTTP_METHODS.inject({}) { |h, m| h[m] = h[m.upcase] = m.to_sym; h }
 
@@ -242,7 +268,6 @@ EOM
     end
     memoize :server_software
 
-
     # Returns the complete URL used for this request.
     def url
       protocol + host_with_port + request_uri
@@ -326,11 +351,7 @@ EOM
 
     # Returns the query string, accounting for server idiosyncrasies.
     def query_string
-      if uri = @env['REQUEST_URI']
-        uri.split('?', 2)[1] || ''
-      else
-        @env['QUERY_STRING'] || ''
-      end
+      @env['QUERY_STRING'].present? ? @env['QUERY_STRING'] : (@env['REQUEST_URI'].split('?', 2)[1] || '')
     end
     memoize :query_string
 
@@ -432,24 +453,36 @@ EOM
       @request_parameters ||= parse_formatted_request_parameters
     end
 
-    #--
-    # Must be implemented in the concrete request
-    #++
-
     def body_stream #:nodoc:
+      @env['rack.input']
     end
 
-    def cookies #:nodoc:
+    def cookies
+      Rack::Request.new(@env).cookies
     end
 
-    def session #:nodoc:
+    def session
+      @env['rack.session'] ||= {}
     end
 
     def session=(session) #:nodoc:
       @session = session
     end
 
-    def reset_session #:nodoc:
+    def reset_session
+      @env['rack.session'] = {}
+    end
+
+    def session_options
+      @env['rack.session.options'] ||= {}
+    end
+
+    def session_options=(options)
+      @env['rack.session.options'] = options
+    end
+
+    def server_port
+      @env['SERVER_PORT'].to_i
     end
 
     protected
@@ -859,66 +892,4 @@ EOM
   class UploadedTempfile < Tempfile
     include UploadedFile
   end
-
-  class Request < AbstractRequest #:nodoc:
-    attr_accessor :session_options
-
-    class SessionFixationAttempt < StandardError #:nodoc:
-    end
-
-    %w[ AUTH_TYPE GATEWAY_INTERFACE PATH_INFO
-        PATH_TRANSLATED REMOTE_HOST
-        REMOTE_IDENT REMOTE_USER SCRIPT_NAME
-        SERVER_NAME SERVER_PROTOCOL
-
-        HTTP_ACCEPT HTTP_ACCEPT_CHARSET HTTP_ACCEPT_ENCODING
-        HTTP_ACCEPT_LANGUAGE HTTP_CACHE_CONTROL HTTP_FROM
-        HTTP_NEGOTIATE HTTP_PRAGMA HTTP_REFERER HTTP_USER_AGENT ].each do |env|
-      define_method(env.sub(/^HTTP_/n, '').downcase) do
-        @env[env]
-      end
-    end
-
-    def query_string
-      qs = super
-      if !qs.blank?
-        qs
-      else
-        @env['QUERY_STRING']
-      end
-    end
-
-    def body_stream #:nodoc:
-      @env['rack.input']
-    end
-
-    def key?(key)
-      @env.key?(key)
-    end
-
-    def cookies
-      Rack::Request.new(@env).cookies
-    end
-
-    def server_port
-      @env['SERVER_PORT'].to_i
-    end
-
-    def session_options
-      @env['rack.session.options'] ||= {}
-    end
-
-    def session_options=(options)
-      @env['rack.session.options'] = options
-    end
-
-    def session
-      @env['rack.session'] ||= {}
-    end
-
-    def reset_session
-      @env['rack.session'] = {}
-    end
-  end
-
 end
diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb
index 694771dfe8..c2c35ad6fe 100644
--- a/actionpack/test/controller/request_test.rb
+++ b/actionpack/test/controller/request_test.rb
@@ -426,95 +426,95 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase
   def test_query_string
     assert_equal(
       { "action" => "create_customer", "full_name" => "David Heinemeier Hansson", "customerId" => "1"},
-      ActionController::AbstractRequest.parse_query_parameters(@query_string)
+      ActionController::Request.parse_query_parameters(@query_string)
     )
   end
 
   def test_deep_query_string
     expected = {'x' => {'y' => {'z' => '10'}}}
-    assert_equal(expected, ActionController::AbstractRequest.parse_query_parameters('x[y][z]=10'))
+    assert_equal(expected, ActionController::Request.parse_query_parameters('x[y][z]=10'))
   end
 
   def test_deep_query_string_with_array
-    assert_equal({'x' => {'y' => {'z' => ['10']}}}, ActionController::AbstractRequest.parse_query_parameters('x[y][z][]=10'))
-    assert_equal({'x' => {'y' => {'z' => ['10', '5']}}}, ActionController::AbstractRequest.parse_query_parameters('x[y][z][]=10&x[y][z][]=5'))
+    assert_equal({'x' => {'y' => {'z' => ['10']}}}, ActionController::Request.parse_query_parameters('x[y][z][]=10'))
+    assert_equal({'x' => {'y' => {'z' => ['10', '5']}}}, ActionController::Request.parse_query_parameters('x[y][z][]=10&x[y][z][]=5'))
   end
 
   def test_deep_query_string_with_array_of_hash
-    assert_equal({'x' => {'y' => [{'z' => '10'}]}}, ActionController::AbstractRequest.parse_query_parameters('x[y][][z]=10'))
-    assert_equal({'x' => {'y' => [{'z' => '10', 'w' => '10'}]}}, ActionController::AbstractRequest.parse_query_parameters('x[y][][z]=10&x[y][][w]=10'))
+    assert_equal({'x' => {'y' => [{'z' => '10'}]}}, ActionController::Request.parse_query_parameters('x[y][][z]=10'))
+    assert_equal({'x' => {'y' => [{'z' => '10', 'w' => '10'}]}}, ActionController::Request.parse_query_parameters('x[y][][z]=10&x[y][][w]=10'))
   end
 
   def test_deep_query_string_with_array_of_hashes_with_one_pair
-    assert_equal({'x' => {'y' => [{'z' => '10'}, {'z' => '20'}]}}, ActionController::AbstractRequest.parse_query_parameters('x[y][][z]=10&x[y][][z]=20'))
-    assert_equal("10", ActionController::AbstractRequest.parse_query_parameters('x[y][][z]=10&x[y][][z]=20')["x"]["y"].first["z"])
-    assert_equal("10", ActionController::AbstractRequest.parse_query_parameters('x[y][][z]=10&x[y][][z]=20').with_indifferent_access[:x][:y].first[:z])
+    assert_equal({'x' => {'y' => [{'z' => '10'}, {'z' => '20'}]}}, ActionController::Request.parse_query_parameters('x[y][][z]=10&x[y][][z]=20'))
+    assert_equal("10", ActionController::Request.parse_query_parameters('x[y][][z]=10&x[y][][z]=20')["x"]["y"].first["z"])
+    assert_equal("10", ActionController::Request.parse_query_parameters('x[y][][z]=10&x[y][][z]=20').with_indifferent_access[:x][:y].first[:z])
   end
 
   def test_deep_query_string_with_array_of_hashes_with_multiple_pairs
     assert_equal(
       {'x' => {'y' => [{'z' => '10', 'w' => 'a'}, {'z' => '20', 'w' => 'b'}]}},
-      ActionController::AbstractRequest.parse_query_parameters('x[y][][z]=10&x[y][][w]=a&x[y][][z]=20&x[y][][w]=b')
+      ActionController::Request.parse_query_parameters('x[y][][z]=10&x[y][][w]=a&x[y][][z]=20&x[y][][w]=b')
     )
   end
 
   def test_query_string_with_nil
     assert_equal(
       { "action" => "create_customer", "full_name" => ''},
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_with_empty)
+      ActionController::Request.parse_query_parameters(@query_string_with_empty)
     )
   end
 
   def test_query_string_with_array
     assert_equal(
       { "action" => "create_customer", "selected" => ["1", "2", "3"]},
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_with_array)
+      ActionController::Request.parse_query_parameters(@query_string_with_array)
     )
   end
 
   def test_query_string_with_amps
     assert_equal(
       { "action" => "create_customer", "name" => "Don't & Does"},
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_with_amps)
+      ActionController::Request.parse_query_parameters(@query_string_with_amps)
     )
   end
 
   def test_query_string_with_many_equal
     assert_equal(
       { "action" => "create_customer", "full_name" => "abc=def=ghi"},
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_with_many_equal)
+      ActionController::Request.parse_query_parameters(@query_string_with_many_equal)
     )
   end
 
   def test_query_string_without_equal
     assert_equal(
       { "action" => nil },
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_without_equal)
+      ActionController::Request.parse_query_parameters(@query_string_without_equal)
     )
   end
 
   def test_query_string_with_empty_key
     assert_equal(
       { "action" => "create_customer", "full_name" => "David Heinemeier Hansson" },
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_with_empty_key)
+      ActionController::Request.parse_query_parameters(@query_string_with_empty_key)
     )
   end
 
   def test_query_string_with_many_ampersands
     assert_equal(
       { "action" => "create_customer", "full_name" => "David Heinemeier Hansson"},
-      ActionController::AbstractRequest.parse_query_parameters(@query_string_with_many_ampersands)
+      ActionController::Request.parse_query_parameters(@query_string_with_many_ampersands)
     )
   end
 
   def test_unbalanced_query_string_with_array
    assert_equal(
      {'location' => ["1", "2"], 'age_group' => ["2"]},
-  ActionController::AbstractRequest.parse_query_parameters("location[]=1&location[]=2&age_group[]=2")
+  ActionController::Request.parse_query_parameters("location[]=1&location[]=2&age_group[]=2")
    )
    assert_equal(
      {'location' => ["1", "2"], 'age_group' => ["2"]},
-     ActionController::AbstractRequest.parse_request_parameters({'location[]' => ["1", "2"],
+     ActionController::Request.parse_request_parameters({'location[]' => ["1", "2"],
   'age_group[]' => ["2"]})
    )
   end
@@ -527,7 +527,7 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase
 
     expected = { "note" => { "viewers"=>{"viewer"=>[{ "id"=>"1", "type"=>"User"}, {"type"=>"Group", "id"=>"2"} ]} } }
 
-    assert_equal(expected, ActionController::AbstractRequest.parse_request_parameters(query))
+    assert_equal(expected, ActionController::Request.parse_request_parameters(query))
   end
 
   def test_parse_params
@@ -566,7 +566,7 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase
       }
     }
 
-    assert_equal expected_output, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected_output, ActionController::Request.parse_request_parameters(input)
   end
 
   UploadedStringIO = ActionController::UploadedStringIO
@@ -621,7 +621,7 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase
       "text_part" => "abc"
     }
 
-    params = ActionController::AbstractRequest.parse_request_parameters(input)
+    params = ActionController::Request.parse_request_parameters(input)
     assert_equal expected_output, params
 
     # Lone filenames are preserved.
@@ -652,7 +652,7 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase
       "logo" => File.new(File.dirname(__FILE__) + "/rack_test.rb").path,
     }
 
-    assert_equal expected_output, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected_output, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_array
@@ -660,55 +660,55 @@ class UrlEncodedRequestParameterParsingTest < ActiveSupport::TestCase
 
     expected_output = { "selected" => [ "1", "2", "3" ] }
 
-    assert_equal expected_output, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected_output, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_non_alphanumeric_name
     input     = { "a/b[c]" =>  %w(d) }
     expected  = { "a/b" => { "c" => "d" }}
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_single_brackets_in_middle
     input     = { "a/b[c]d" =>  %w(e) }
     expected  = { "a/b" => {} }
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_separated_brackets
     input     = { "a/b@[c]d[e]" =>  %w(f) }
     expected  = { "a/b@" => { }}
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_separated_brackets_and_array
     input     = { "a/b@[c]d[e][]" =>  %w(f) }
     expected  = { "a/b@" => { }}
-    assert_equal expected , ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected , ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_unmatched_brackets_and_array
     input     = { "a/b@[c][d[e][]" =>  %w(f) }
     expected  = { "a/b@" => { "c" => { }}}
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_nil_key
     input = { nil => nil, "test2" => %w(value1) }
     expected = { "test2" => "value1" }
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_array_prefix_and_hashes
     input = { "a[][b][c]" => %w(d) }
     expected = {"a" => [{"b" => {"c" => "d"}}]}
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 
   def test_parse_params_with_complex_nesting
     input = { "a[][b][c][][d][]" => %w(e) }
     expected = {"a" => [{"b" => {"c" => [{"d" => ["e"]}]}}]}
-    assert_equal expected, ActionController::AbstractRequest.parse_request_parameters(input)
+    assert_equal expected, ActionController::Request.parse_request_parameters(input)
   end
 end
 
@@ -770,7 +770,7 @@ class MultipartRequestParameterParsingTest < ActiveSupport::TestCase
       # Ensures that parse_multipart_form_parameters works with streams that cannot be rewound
       file = File.open(File.join(FIXTURE_PATH, 'large_text_file'), 'rb')
       file.expects(:rewind).raises(Errno::ESPIPE)
-      params = ActionController::AbstractRequest.parse_multipart_form_parameters(file, 'AaB03x', file.stat.size, {})
+      params = ActionController::Request.parse_multipart_form_parameters(file, 'AaB03x', file.stat.size, {})
       assert_not_equal 0, file.pos  # file was not rewound after reading
     end
   end
@@ -809,7 +809,7 @@ class MultipartRequestParameterParsingTest < ActiveSupport::TestCase
   private
     def parse_multipart(name)
       File.open(File.join(FIXTURE_PATH, name), 'rb') do |file|
-        params = ActionController::AbstractRequest.parse_multipart_form_parameters(file, 'AaB03x', file.stat.size, {})
+        params = ActionController::Request.parse_multipart_form_parameters(file, 'AaB03x', file.stat.size, {})
         assert_equal 0, file.pos  # file was rewound after reading
         params
       end