Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't error when size is called on a deleted file #1561

Conversation

danielevans
Copy link
Contributor

When using Fog and AWS and a new file is uploaded and the old file is missing content_length is called on nil, resulting in the inability to upload a new file.

This change updates the CarrierWave::Storage::Fog class such that it will not error if this situation occurs and instead return a 0 file size.

It seems that this change could also be made in https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/sanitized_file.rb#L136 by reversing the conditional (self.size.zero? && ! self.exists?), however the change in this PR seemed less invasive.

The error caused by this issue:

NoMethodError: undefined method `content_length' for nil:NilClass
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/storage/fog.rb:291:in `size'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/uploader/proxy.rb:57:in `size'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/sanitized_file.rb:96:in `size'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/sanitized_file.rb:136:in `empty?'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/uploader/cache.rb:111:in `cache!'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/mounter.rb:43:in `block in cache'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/mounter.rb:41:in `map'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/mounter.rb:41:in `cache'
    from /srv/app/releases/20150213002036/vendor/bundle/ruby/2.1.0/bundler/gems/carrierwave-0033279be9b6/lib/carrierwave/mount.rb:148:in `avatar='

@bhavinjavia
Copy link

+1. we're running into this issue as well

bensie added a commit that referenced this pull request Feb 26, 2015
…evious_file

Don't error when size is called on a deleted file
@bensie bensie merged commit bbef19c into carrierwaveuploader:master Feb 26, 2015
@bensie
Copy link
Member

bensie commented Feb 26, 2015

Thanks @danielevans!

@ghost
Copy link

ghost commented Mar 6, 2015

we are running too, thanks

@ghost
Copy link

ghost commented May 6, 2015

I just got

undefined method `content_length' for nil:NilClass

When updating an image, i'm using latest carrierwave with rails 4.

Thanks!

@danielevans
Copy link
Contributor Author

@amartell-smx Looks like this didn't make it into the 0.10.0 release. Have you tried using the latest master branch?

@ghost
Copy link

ghost commented May 6, 2015

Got this message

/Users/abimael/.rbenv/versions/2.1.4/lib/ruby/gems/2.1.0/bundler/gems/carrierwave-45d90749693f/lib/carrierwave/uploader/configu
ration.rb:75:in `eval': uninitialized constant CarrierWave::Storage::Fog (NameError)

With master, still i don't want to use master version on production, is there any way to workaround the size issue?

Thanks!

@danielevans
Copy link
Contributor Author

You could easily monkey-patch the fix in.
The following code could go in lib/ext/carrierwave/storage/fog.rb and required the file from config/application.rb or an initializer.

require 'carrierwave/storage/fog'

class CarrierWave::Storage::Fog::File
  def size
    file.nil? ? 0 : file.content_length
  end
end

@ghost
Copy link

ghost commented May 7, 2015

Actually it should be

require 'carrierwave/storage/fog'

class CarrierWave::Storage::Fog::File
  def size
    file.nil? ? 0 : file.content_length
  end
end

Thanks!

@danielevans
Copy link
Contributor Author

Gah, thanks. I edited mine to make sure nobody copy-pastes the bad code.

@jvenezia
Copy link
Contributor

jvenezia commented Apr 7, 2016

Still not in the lastest release (0.11.0). Is there a reason not to release this ? I'll monkey-patch for the moment.

@jvenezia
Copy link
Contributor

jvenezia commented Apr 7, 2016

Same problem with content_type: #1915

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants