From 52ca5dad1ecf64ff508a4f16202171693921890f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sat, 6 Oct 2007 11:40:13 +0000 Subject: Use StringIO and Tempfile subclasses instead of defining singleton methods on each multipart field. git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7759 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- actionpack/lib/action_controller/request.rb | 106 ++++++++++++++++------------ actionpack/test/controller/request_test.rb | 32 +++++---- 2 files changed, 79 insertions(+), 59 deletions(-) diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb index e9a2e797dd..1fa6accd01 100755 --- a/actionpack/lib/action_controller/request.rb +++ b/actionpack/lib/action_controller/request.rb @@ -459,36 +459,16 @@ module ActionController when Array value.map { |v| get_typed_value(v) } else - # This is an uploaded file. - if value.respond_to?(:original_filename) && !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 - end - end - end + if value.is_a?(UploadedFile) + # Uploaded file + if value.original_filename + value + # Multipart param + else + result = value.read + value.rewind + result 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}" @@ -524,9 +504,9 @@ module ActionController head = nil content = if 10240 < content_length - Tempfile.new("CGI") + UploadedTempfile.new("CGI") else - StringIO.new + UploadedStringIO.new end content.binmode if defined? content.binmode @@ -568,25 +548,21 @@ module ActionController content.rewind - /Content-Disposition:.* filename=(?:"((?:\\.|[^\"])*)"|([^;]*))/ni.match(head) - filename = ($1 or $2 or "") - if /Mac/ni.match(env['HTTP_USER_AGENT']) and - /Mozilla/ni.match(env['HTTP_USER_AGENT']) and - (not /MSIE/ni.match(env['HTTP_USER_AGENT'])) - filename = CGI.unescape(filename) + head =~ /Content-Disposition:.* filename=(?:"((?:\\.|[^\"])*)"|([^;]*))/ni + if filename = $1 || $2 + if /Mac/ni.match(env['HTTP_USER_AGENT']) and + /Mozilla/ni.match(env['HTTP_USER_AGENT']) and + (not /MSIE/ni.match(env['HTTP_USER_AGENT'])) + filename = CGI.unescape(filename) + end + content.original_path = filename.dup end - /Content-Type: ([^\r]*)/ni.match(head) - content_type = ($1 or "") + head =~ /Content-Type: ([^\r]*)/ni + content.content_type = $1.dup if $1 - (class << content; self; end).class_eval do - alias local_path path - define_method(:original_filename) {filename.dup.taint} - define_method(:content_type) {content_type.dup.taint} - end - - /Content-Disposition:.* name="?([^\";]*)"?/ni.match(head) - name = $1.dup + head =~ /Content-Disposition:.* name="?([^\";]*)"?/ni + name = $1.dup if $1 if params.has_key?(name) params[name].push(content) @@ -695,4 +671,40 @@ module ActionController raise TypeError, "Conflicting types for parameter containers. Expected an instance of #{klass} but found an instance of #{value.class}. This can be caused by colliding Array and Hash parameters like qs[]=value&qs[key]=value." end end + + module UploadedFile + def self.included(base) + base.class_eval do + attr_accessor :original_path, :content_type + alias_method :local_path, :path + end + end + + # 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 + unless defined? @original_filename + @original_filename = + unless original_path.blank? + if original_path =~ /^(?:.*[:\\\/])?(.*)/m + $1 + else + File.basename original_path + end + end + end + @original_filename + end + end + + class UploadedStringIO < StringIO + include UploadedFile + end + + class UploadedTempfile < Tempfile + include UploadedFile + end end diff --git a/actionpack/test/controller/request_test.rb b/actionpack/test/controller/request_test.rb index c62b2a6ec7..922699e331 100644 --- a/actionpack/test/controller/request_test.rb +++ b/actionpack/test/controller/request_test.rb @@ -527,21 +527,29 @@ class UrlEncodedRequestParameterParsingTest < Test::Unit::TestCase assert_equal expected_output, ActionController::AbstractRequest.parse_request_parameters(input) end + UploadedStringIO = ActionController::UploadedStringIO + class MockUpload < UploadedStringIO + def initialize(content_type, original_path, *args) + self.content_type = content_type + self.original_path = original_path + super *args + end + end + def test_parse_params_from_multipart_upload - 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') + file = MockUpload.new('img/jpeg', 'foo.jpg') + ie_file = MockUpload.new('img/jpeg', 'c:\\Documents and Settings\\foo\\Desktop\\bar.jpg') + non_file_text_part = MockUpload.new('text/plain', '', 'abc') input = { - "something" => [ StringIO.new("") ], - "array_of_stringios" => [[ StringIO.new("One"), StringIO.new("Two") ]], - "mixed_types_array" => [[ StringIO.new("Three"), "NotStringIO" ]], - "mixed_types_as_checkboxes[strings][nested]" => [[ file, "String", StringIO.new("StringIO")]], - "ie_mixed_types_as_checkboxes[strings][nested]" => [[ ie_file, "String", StringIO.new("StringIO")]], - "products[string]" => [ StringIO.new("Apple Computer") ], + "something" => [ UploadedStringIO.new("") ], + "array_of_stringios" => [[ UploadedStringIO.new("One"), UploadedStringIO.new("Two") ]], + "mixed_types_array" => [[ UploadedStringIO.new("Three"), "NotStringIO" ]], + "mixed_types_as_checkboxes[strings][nested]" => [[ file, "String", UploadedStringIO.new("StringIO")]], + "ie_mixed_types_as_checkboxes[strings][nested]" => [[ ie_file, "String", UploadedStringIO.new("StringIO")]], + "products[string]" => [ UploadedStringIO.new("Apple Computer") ], "products[file]" => [ file ], - "ie_products[string]" => [ StringIO.new("Microsoft") ], + "ie_products[string]" => [ UploadedStringIO.new("Microsoft") ], "ie_products[file]" => [ ie_file ], "text_part" => [non_file_text_part] } @@ -695,7 +703,7 @@ class MultipartRequestParameterParsingTest < Test::Unit::TestCase file = params['file'] assert_kind_of StringIO, file assert_equal 'file.csv', file.original_filename - assert_equal '', file.content_type + assert_nil file.content_type assert_equal 'contents', file.read file = params['flowers'] -- cgit v1.2.3