34

I inherited a Rails 2.2.2 app that stores user-uploaded images on Amazon S3. The attachment_fu-based Photo model offers a rotate method that uses open-uri to retrieve the image from S3 and MiniMagick to perform the rotation.

The rotate method contains this line to retrieve the image for use with MiniMagick:

temp_image = MiniMagick::Image.from_file(open(self.public_filename).path)

self.public_filename returns something like 

http://s3.amazonaws.com/bucketname/photos/98/photo.jpg

Retrieving the image and rotating it work just fine in the running application in production and development. However, the unit test fails with

TypeError: can't convert nil into String
    /Users/santry/Development/totspot/vendor/gems/mini_magick-1.2.3/lib/mini_magick.rb:34:in `initialize'
    /Users/santry/Development/totspot/vendor/gems/mini_magick-1.2.3/lib/mini_magick.rb:34:in `open'
    /Users/santry/Development/totspot/vendor/gems/mini_magick-1.2.3/lib/mini_magick.rb:34:in `from_file'

The reason is that when the model method is called in the context of the unit test, open(self.public_filename) is returning a StringIO object that contains the image data. The path method on this object returns nil and MiniMagick::Image.from_file blows up.

When this very same model method is called from the PhotosController, open(self.public_filename) returns a FileIO instance tied to a file named, eg, /tmp/open-uri7378-0 and the file contains the image data.

Thinking the cause must be some environmental difference between test and development, I fired up the console under the development environment. But just as in the unit test, open('http://...') returned a StringIO, not a FileIO.

I've traced my way through open-uri and all the relevant application-specific code and can find no reason for the difference.

Andrew Grimm
  • 78,473
  • 57
  • 200
  • 338
Sean Santry
  • 343
  • 1
  • 3
  • 4

3 Answers3

79

The open-uri library uses a constant to set the 10KB size limit for StringIO objects.

> OpenURI::Buffer::StringMax
=> 10240

You can change this setting to 0 to prevent open-uri from ever creating a StringIO object. Instead, this will force it to always generate a temp file.

Just throw this in an initializer:

# Don't allow downloaded files to be created as StringIO. Force a tempfile to be created.
require 'open-uri'
OpenURI::Buffer.send :remove_const, 'StringMax' if OpenURI::Buffer.const_defined?('StringMax')
OpenURI::Buffer.const_set 'StringMax', 0

You can't just set the constant directly. You need to actually remove the constant and then set it again (as above), otherwise you'll get a warning: 

warning: already initialized constant StringMax

UPDATED 12/18/2012: Rails 3 doesn't require OpenURI by default, so you need to add require 'open-uri' at the top of the initializer. I updated the code above to reflect that change.

Micah Winkelspecht
  • 1,805
  • 1
  • 16
  • 7
  • Micah, This looks great but I'm getting errors when I try it in a Rails 3 app. It looks like a Buffer object hasn't been created yet. uninitialized constant OpenURI (NameError) – Paul Oct 25 '12 at 09:17
  • Thanks, Paul, I updated the code in the answer. Thanks, Michael, I love you too. – Micah Winkelspecht Dec 19 '12 at 01:02
  • 1
    Changing this application-wide, in the initializer, what performance implications does it have? – Marius Butuc Sep 08 '15 at 17:22
  • Thanks for this solution. Came here from CSV error `no implicit conversion of StringIO into String` – goodmanship Apr 05 '16 at 21:40
  • This has been driving me nuts for a couple weeks now. Thanks for this answer! – Phatjam98 Jun 12 '17 at 22:10
  • padame loves you bro – Pacolotero Jun 20 '19 at 12:29
  • 1
    Amazing answer that has stood the test of time. Firm handshakes, Micah. – TheBrockEllis Apr 19 '23 at 19:09
  • Yeah the 10kb threshold caused a production bug in our app; forcing `OpenURI` to always create a temp file definitely did the trick. If the file being read was < 10kb it would create a `StringIO` object which was not handled by our backend nicely. HUGE props for your answer here, lifesaver. Also, who the f decided to implement this threshold and change the returned object type? Very subtle bug-inducing threshold... the `open` method really shouldn't change behavior based on the file size being read... but that's just my 2¢ tho! – Chris Cirefice May 25 '23 at 18:17
31

The code responsible for this is in the Buffer class in open-uri. It starts by creating a StringIO object and only creates an actual temp file in the local filesystem when the data exceeds a certain size (10 KB).

I assume that whatever data your test is loading is small enough to be held in a StringIO and the images you are using in the actual application are large enough to warrant a TempFile. The solution is to use methods which are common to both classes, in particular the read method, with MiniMagick::Image#from_blob:

temp_image = MiniMagick::Image.from_blob(open(self.public_filename, &:read))
Nanda
  • 464
  • 4
  • 6
Jeff Dallien
  • 4,491
  • 3
  • 22
  • 19
  • 3
    Don't do `open(self.public_filename).read`, you don't know when the handle will be closed. Use `open(self.public_filename, &:read)` instead, which uses block-form and explicitly closes when done. And it's not really more code. – apeiros Feb 21 '13 at 22:29
  • FYI, `from_blob` is now deprecated in favor of `read`. See https://github.com/probablycorey/mini_magick/blob/f309fbf390cd21a845264bca9bec95b9bdae8029/lib/mini_magick.rb#L82 – Jessie A. Young Sep 06 '17 at 20:58
0

Now OpenURI::Buffer::StringMax can be set directly:

require 'open-uri'
OpenURI::Buffer::StringMax = 0

but with a warning:

warning: already initialized constant OpenURI::Buffer::StringMax
Jinbo
  • 417
  • 3
  • 10