Multipart form values may have a content type without being treated as uploaded files if they do not provide a filename. Closes #6401.

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

actionpack/lib/action_controller/cgi_ext/cgi_methods.rb

@@ -63,43 +63,50 @@ class CGIMethods #:nodoc:
 
     private
       def get_typed_value(value)
-        # test most frequent case first
-        if value.is_a?(String)
-          value
-        elsif value.respond_to?(:content_type) && ! value.content_type.blank?
-          # Uploaded file
-          unless value.respond_to?(:full_original_filename)
-            class << value
-              alias_method :full_original_filename, :original_filename
+        case value
+          when String
+            value
+          when NilClass
+            ''
+          when Array
+            value.map { |v| get_typed_value(v) }
+          else
+            # Uploaded file provides content type and filename.
+            if value.respond_to?(:content_type) &&
+                  !value.content_type.blank? &&
+                  !value.original_filename.blank?
+              unless value.respond_to?(:full_original_filename)
+                class << value
+                  alias_method :full_original_filename, :original_filename
 
-              # Take the basename of the upload's original filename.
-              # This handles the full Windows paths given by Internet Explorer
-              # (and perhaps other broken user agents) without affecting
-              # those which give the lone filename.
-              # The Windows regexp is adapted from Perl's File::Basename.
-              def original_filename
-                if md = /^(?:.*[:\\\/])?(.*)/m.match(full_original_filename)
-                  md.captures.first
-                else
-                  File.basename full_original_filename
+                  # Take the basename of the upload's original filename.
+                  # This handles the full Windows paths given by Internet Explorer
+                  # (and perhaps other broken user agents) without affecting
+                  # those which give the lone filename.
+                  # The Windows regexp is adapted from Perl's File::Basename.
+                  def original_filename
+                    if md = /^(?:.*[:\\\/])?(.*)/m.match(full_original_filename)
+                      md.captures.first
+                    else
+                      File.basename full_original_filename
+                    end
+                  end
                 end
               end
+
+              # Return the same value after overriding original_filename.
+              value
+
+            # Multipart values may have content type, but no filename.
+            elsif value.respond_to?(:read)
+              result = value.read
+              value.rewind
+              result
+
+            # Unknown value, neither string nor multipart.
+            else
+              raise "Unknown form value: #{value.inspect}"
             end
-          end
-
-          # Return the same value after overriding original_filename.
-          value
-
-        elsif value.respond_to?(:read)
-          # Value as part of a multipart request
-          result = value.read
-          value.rewind
-          result
-        elsif value.class == Array
-          value.collect { |v| get_typed_value(v) }
-        else
-          # other value (neither string nor a multipart request)
-          value.to_s
         end
       end
   end

actionpack/test/controller/cgi_test.rb

@@ -155,9 +155,10 @@ class CGITest < Test::Unit::TestCase
   end
   
   def test_parse_params_from_multipart_upload
-    mockup = Struct.new(:content_type, :original_filename)
+    mockup = Struct.new(:content_type, :original_filename, :read, :rewind)
     file = mockup.new('img/jpeg', 'foo.jpg')
     ie_file = mockup.new('img/jpeg', 'c:\\Documents and Settings\\foo\\Desktop\\bar.jpg')
+    non_file_text_part = mockup.new('text/plain', '', 'abc')
   
     input = {
       "something" => [ StringIO.new("") ],
@@ -168,9 +169,10 @@ class CGITest < Test::Unit::TestCase
       "products[string]" => [ StringIO.new("Apple Computer") ],
       "products[file]" => [ file ],
       "ie_products[string]" => [ StringIO.new("Microsoft") ],
-      "ie_products[file]" => [ ie_file ]
+      "ie_products[file]" => [ ie_file ],
+      "text_part" => [non_file_text_part]
     }
-    
+
     expected_output =  {
       "something" => "",
       "array_of_stringios" => ["One", "Two"],
@@ -192,7 +194,8 @@ class CGITest < Test::Unit::TestCase
       "ie_products" => {
         "string" => "Microsoft",
         "file" => ie_file
-      }
+      },
+      "text_part" => "abc"
     }
 
     params = CGIMethods.parse_request_parameters(input)
@@ -338,7 +341,7 @@ class MultipartCGITest < Test::Unit::TestCase
     assert_equal 'bar', params['foo']
 
     # Ruby CGI doesn't handle multipart/mixed for us.
-    assert_kind_of StringIO, params['files']
+    assert_kind_of String, params['files']
     assert_equal 19756, params['files'].size
   end