Why does using OpenURI to download a file result in a partial file?

Question

I'm trying to use OpenURI to download a file from S3, and then save it locally so I can send the file as an attachment with ActionMailer.

Something strange is going on. The images being downloaded and attached are corrupt, the bottom parts of the images are missing.

Here's the code:

require 'open-uri'
open("#{Rails.root.to_s}/tmp/#{a.attachment_file_name}", "wb") do |file|  
  source_url = a.authenticated_url()
  io = open(URI.parse(source_url).to_s)
  file << io.read
  attachments[a.attachment_file_name] = File.read("#{Rails.root.to_s}/tmp/#{a.attachment_file_name}")        
end

a is the attachment from ActionMailer.

What can I try next?

Answer 1

It looks like you're trying to read the file before it's been closed, which could leave part of the file buffer unwritten.

I'd do it like this:

require 'open-uri'

source_url = a.authenticated_url()
attachment_file = "#{Rails.root.to_s}/tmp/#{a.attachment_file_name}"
open(attachment_file, "wb") do |file|  
  file.print open(source_url, &:read)
end

attachments[a.attachment_file_name] = File.read(attachment_file)

It looks like source_url = a.authenticated_url() will be a string, so parsing the string into a URI then doing to_s on it will be redundant unless URI is doing some normalizing, which I don't think it does.

Based on my sysadmin experience: A side task is cleaning up the downloaded/spooled files. They could be deleted immediately after being attached, or you could have a cron job that runs daily, deleting all spooled files over one day old.

An additional concern for this is there is no error handling in case the URL can't be read, causing the attachment to fail. Using a temp spool file you could check for the existence of the file. Even better, you should probably be prepared to handle an exception if the server returns a 400 or 500 error.

---

To avoid using a temporary spool file try this untested code:

require 'open-uri'

source_url = a.authenticated_url()
attachments[a.attachment_file_name] = open(source_url, &:read)