CGI uses file size to distinguish between regular values and files

ruby-talk
21

In article 1067952745.419615.23337.nullmailer@picachu.netlab.jp,
matz@ruby-lang.org (Yukihiro Matsumoto) writes:

How about

require ‘fileutils’
File.open(upload, ‘w’) {|f| FileUtils.copy_stream(file,f)}

? It’s bit longer, but works now without any modifies.

I see. It is a solution but I found that I misunderstand the problem.

I thought that we cannot assume cgi[‘file’] has an actual file. But
the assumption was correct in ruby-1.6 because always cgi.rb use
Tempfile since StringIO was not bundled.

It is documented in cgi.rb:

=== Get multipart form values

···

require “cgi”

cgi = CGI.new

value = cgi[‘field_name’] # <== value string for ‘field_name’

value.read # <== body of value

value.local_path # <== path to local file of value

value.original_filename # <== original filename of value

value.content_type # <== content_type of value

The existence of the method `local_path’ means that there is an actual
file in somewere.

But now local_path returns invalid pathname such as
“StringIO:0x401fe6d8” for smaller file.

So the problem is an incompatibility between ruby-1.6 and ruby-1.8.

It can be solved by:

  • obsolete `local_path’ and update documentation,
  • don’t use StringIO or
  • switch StringIO to Tempfile when local_path is called.
    –
    Tanaka Akira
22

That is better than my ftools-based solution, since it solves the
problem of differentiating between StringIO and Tempfile (although it
won’t work with Ruby 1.6, if anyone still cares about that).

But I think the harder problem lies earlier in the execution path, when
application decides whether to handle the parameter value as a String or
as IO. I don’t see any simple way of moving responsibility for this
decision from application to cgi.rb library without getting bitten by
the corner case of a large upload that wouldn’t fit into memory.

That is, unless Ruby learns to automatically convert strings to
tempfiles as soon as they become too large, which would be quite a
feat… :slight_smile:

In the absence of such feat, I think it would be nice to have

def multipart?
@multipart
end

in class CGI in order to make detection of multipart data more obvious.

···

On Wed, Nov 05, 2003 at 12:53:01AM +0900, Yukihiro Matsumoto wrote:

How about

require ‘fileutils’
File.open(upload, ‘w’) {|f| FileUtils.copy_stream(file,f)}

? It’s bit longer, but works now without any modifies.

–
Dmitry Borodaenko

23

So the problem is an incompatibility between ruby-1.6 and ruby-1.8.

It can be solved by:

  • obsolete `local_path’ and update documentation,
  • don’t use StringIO or
  • switch StringIO to Tempfile when local_path is called.

I like the last solution better by far. You’d be able to keep most
values as StringIO (in memory). It wouldn’t break 1.6.x code that
depends on the old behaviour. And it’s transparent.

Mind you, I still think this is patching a somewhat questionable
behaviour, but at least it would work! :slight_smile:

···

–
David Heinemeier Hansson.
http://www.loudthinking.com/ – Broadcasting Brain