Cope with missing content type and length headers. Parse parameters from multipart and urlencoded request bodies only. Accept multipart PUT parameters. Closes #5235.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4388 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -1,5 +1,9 @@
 *SVN*
 
+* Cope with missing content type and length headers. Parse parameters from multipart and urlencoded request bodies only. [Jeremy Kemper]
+
+* Accept multipart PUT parameters. #5235 [guy.naor@famundo.com]
+
 * Added interrogation of params[:format] to determine Accept type. If :format is specified and matches a declared extension, like "rss" or "xml", that mime type will be put in front of the accept handler. This means you can link to the same action from different extensions and use that fact to determine output [DHH]. Example:
 
   class WeblogController < ActionController::Base

actionpack/lib/action_controller/cgi_ext/raw_post_data_fix.rb

@@ -1,27 +1,48 @@
 class CGI #:nodoc:
-  # Add @request.env['RAW_POST_DATA'] for the vegans.
   module QueryExtension
     # Initialize the data from the query.
     #
     # Handles multipart forms (in particular, forms that involve file uploads).
     # Reads query parameters in the @params field, and cookies into @cookies.
-    def initialize_query()
+    def initialize_query
       @cookies = CGI::Cookie::parse(env_table['HTTP_COOKIE'] || env_table['COOKIE'])
 
-      #fix some strange request environments
+      # Fix some strange request environments.
       if method = env_table['REQUEST_METHOD']
         method = method.to_s.downcase.intern
       else
         method = :get
       end
 
-      if method == :post && (boundary = multipart_form_boundary)
-        @multipart = true
-        @params = read_multipart(boundary, Integer(env_table['CONTENT_LENGTH']))
-      else
-        @multipart = false
-        @params = CGI::parse(read_query_params(method) || "")
+      # POST assumes missing Content-Type is application/x-www-form-urlencoded.
+      content_type = env_table['CONTENT_TYPE']
+      if content_type.blank? && method == :post
+        content_type = 'application/x-www-form-urlencoded'
       end
+
+      # Force content length to zero if missing.
+      content_length = env_table['CONTENT_LENGTH'].to_i
+
+      # Set multipart to false by default.
+      @multipart = false
+
+      # POST and PUT may have params in entity body. If content type is
+      # missing for POST, assume urlencoded. If content type is missing
+      # for PUT, don't assume anything and don't parse the parameters:
+      # it's likely binary data.
+      #
+      # The other HTTP methods have their params in the query string.
+      if method == :post || method == :put
+        if boundary = extract_multipart_form_boundary(content_type)
+          @multipart = true
+          @params = read_multipart(boundary, content_length)
+        elsif content_type.downcase != 'application/x-www-form-urlencoded'
+          read_params(method, content_length)
+          @params = {}
+        end
+      end
+
+      @params ||= CGI.parse(read_params(method, content_length))
     end
 
     private
@@ -29,16 +50,16 @@ class CGI #:nodoc:
         MULTIPART_FORM_BOUNDARY_RE = %r|\Amultipart/form-data.*boundary=\"?([^\";,]+)\"?|n #"
       end
 
-      def multipart_form_boundary
-        MULTIPART_FORM_BOUNDARY_RE.match(env_table['CONTENT_TYPE']).to_a.pop
+      def extract_multipart_form_boundary(content_type)
+        MULTIPART_FORM_BOUNDARY_RE.match(content_type).to_a.pop
       end
 
       if defined? MOD_RUBY
-        def read_params_from_query
+        def read_query
           Apache::request.args || ''
         end
       else
-        def read_params_from_query
+        def read_query
           # fixes CGI querystring parsing for lighttpd
           env_qs = env_table['QUERY_STRING']
           if env_qs.blank? && !(uri = env_table['REQUEST_URI']).blank?
@@ -49,25 +70,25 @@ class CGI #:nodoc:
         end
       end
 
-      def read_params_from_post
+      def read_body(content_length)
         stdinput.binmode if stdinput.respond_to?(:binmode)
-        content = stdinput.read(Integer(env_table['CONTENT_LENGTH'])) || ''
-        # fix for Safari Ajax postings that always append \000
+        content = stdinput.read(content_length) || ''
+        # Fix for Safari Ajax postings that always append \000
         content.chop! if content[-1] == 0
         content.gsub! /&_=$/, ''
         env_table['RAW_POST_DATA'] = content.freeze
       end
 
-      def read_query_params(method)
+      def read_params(method, content_length)
         case method
           when :get
-            read_params_from_query
+            read_query
           when :post, :put
-            read_params_from_post
+            read_body(content_length)
           when :cmd
             read_from_cmdline
-          else # when :head, :delete, :options
-            read_params_from_query
+          else # :head, :delete, :options, :trace, :connect
+            read_query
         end
       end
   end # module QueryExtension

actionpack/test/controller/cgi_test.rb

@@ -235,6 +235,7 @@ class CGITest < Test::Unit::TestCase
   end
 end
 
+
 class MultipartCGITest < Test::Unit::TestCase
   FIXTURE_PATH = File.dirname(__FILE__) + '/../fixtures/multipart'
 
@@ -315,6 +316,14 @@ class MultipartCGITest < Test::Unit::TestCase
     end
 end
 
+# Ensures that PUT works with multipart as well as POST.
+class PutMultipartCGITest < MultipartCGITest
+  def setup
+    super
+    ENV['REQUEST_METHOD'] = 'PUT'
+  end
+end
+
 
 class CGIRequestTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/raw_post_test.rb

@@ -5,27 +5,64 @@ require File.dirname(__FILE__) + '/../../lib/action_controller/cgi_ext/raw_post_
 
 class RawPostDataTest < Test::Unit::TestCase
   def setup
-    ENV['REQUEST_METHOD'] = 'POST'
-    ENV['CONTENT_TYPE'] = ''
-    ENV['CONTENT_LENGTH'] = '0'
+    ENV.delete('RAW_POST_DATA')
+    @request_body = 'a=1'
   end
 
-  def test_raw_post_data
-    process_raw "action=create_customer&full_name=David%20Heinemeier%20Hansson&customerId=1"
+  def test_post_with_urlencoded_body
+    ENV['REQUEST_METHOD'] = 'POST'
+    ENV['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'
+    assert_equal ['1'], cgi_params['a']
+    assert_has_raw_post_data
+  end
+
+  def test_post_with_empty_content_type_treated_as_urlencoded
+    ENV['REQUEST_METHOD'] = 'POST'
+    ENV['CONTENT_TYPE'] = ''
+    assert_equal ['1'], cgi_params['a']
+    assert_has_raw_post_data
+  end
+
+  def test_post_with_unrecognized_content_type_reads_body_but_doesnt_parse_params
+    ENV['REQUEST_METHOD'] = 'POST'
+    ENV['CONTENT_TYPE'] = 'foo/bar'
+    assert cgi_params.empty?
+    assert_has_raw_post_data
+  end
+
+  def test_put_with_urlencoded_body
+    ENV['REQUEST_METHOD'] = 'PUT'
+    ENV['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'
+    assert_equal ['1'], cgi_params['a']
+    assert_has_raw_post_data
+  end
+
+  def test_put_with_empty_content_type_ignores_body
+    ENV['REQUEST_METHOD'] = 'PUT'
+    ENV['CONTENT_TYPE'] = ''
+    assert cgi_params.empty?
+    assert_has_raw_post_data
+  end
+
+  def test_put_with_unrecognized_content_type_ignores_body
+    ENV['REQUEST_METHOD'] = 'PUT'
+    ENV['CONTENT_TYPE'] = 'foo/bar'
+    assert cgi_params.empty?
+    assert_has_raw_post_data
   end
 
   private
-    def process_raw(query_string)
-      old_stdin = $stdin
-      begin
-        $stdin = StringIO.new(query_string.dup)
-        ENV['CONTENT_LENGTH'] = $stdin.size.to_s
-        CGI.new
-        assert_not_nil ENV['RAW_POST_DATA']
-        assert ENV['RAW_POST_DATA'].frozen?
-        assert_equal query_string, ENV['RAW_POST_DATA']
-      ensure
-        $stdin = old_stdin
-      end
+    def cgi_params
+      old_stdin, $stdin = $stdin, StringIO.new(@request_body.dup)
+      ENV['CONTENT_LENGTH'] = $stdin.size.to_s
+      CGI.new.params
+    ensure
+      $stdin = old_stdin
+    end
+
+    def assert_has_raw_post_data(expected_body = @request_body)
+      assert_not_nil ENV['RAW_POST_DATA']
+      assert ENV['RAW_POST_DATA'].frozen?
+      assert_equal expected_body, ENV['RAW_POST_DATA']
     end
 end