darthsim/carrierwave-bombshelter

Protects your carrierwave from image bombs

carrierwave-bombshelter's Issues

Usage of Ruby 2.0 %i()

Unless this gem is intended to be only used for Ruby 2.0 installations and above, the usage of %i() to create an array of symbols in the image_type_whitelist method breaks with earlier versions of Ruby.

BombShelter pollutes callbacks

Greetings,

Discovered this issue after upgrading to the latest version.

I have 2 different uploaders AttachmentUploader and WebsiteUploader. The first one is supposed to accept any kind of files, the second one accept only images files.
I've included BombShelter into WebsiteUploader only.

When my code uses AttachmentUploader, I got the error:

NoMethodError: undefined method `protect_from_image_bomb!' for #<AttachmentUploader:0x007f5ca18e26c8>

If I remove the line include CarrierWave::BombShelter from WebsiteUploader, the error disappear.
I think the way you implement the callback is faulty.

Sorry I have no time to dive deeper, so I leave it to you as a FYI.

Cheers

Cannot assign uploader from one model to another to copy files

This code used to work for us (we mount our Image uploader on the media field, and images are stored on S3 using the fog gem):

copied_asset = Assets::Image.create(
  media: original_asset.media,
  internal_name: "published_#{original_asset.internal_name}"
)

Now it fails to save because of image type protection here:
https://github.com/DarthSim/carrierwave-bombshelter/blob/master/lib/carrierwave/bombshelter.rb#L41
Which generates

copied_asset.errors.full_messages
# => ["Media Image has an unsupported type"]

I may be completely on the wrong track, but when I look at the line that feeds the type checker
https://github.com/DarthSim/carrierwave-bombshelter/blob/master/lib/carrierwave/bombshelter.rb#L36

def protect_from_image_bomb!(new_file)
      image = FastImage.new(new_file.path || get_real_file(new_file.file))

and try to pass in
new_file = original_asset.media,
then since new_file.path resolves to image_assets/3bb19772-4350-4f96-9279-688740bb7628.png, I get

image = FastImage.new(new_file.path || get_real_file(new_file.file))
# => #<FastImage:0x007f9f0a75dcc0 
@uri="image_assets/3bb19772-4350-4f96-9279-688740bb7628.png", 
@options={:type_only=>false, :timeout=>2, :raise_on_failure=>false, :proxy=>nil, :http_header=>{}}, 
@property=:size,
@parsed_uri=#<Addressable::URI:0x3fcf853aebcc URI:image_assets/3bb19772-4350-4f96-9279-688740bb7628.png>
>
image.type
# => nil

If I skip the path option I get

image = FastImage.new( get_real_file(new_file.file))
ArgumentError: wrong number of arguments (1 for 0)
  from .../gems/carrierwave-0.11.1/lib/carrierwave/storage/fog.rb:225:in `read'
  from .../gems/fastimage-2.0.0/lib/fastimage.rb:327:in `block in fetch_using_read'

(which suggests I am sending in the wrong object; though new_file.file responds_to?(:read), which makes FastImage think it can call read(LocalFileChunkSize) on it--which is one too many arguments for Fog )

If I use new_file = original_asset.media.file I still get

image = FastImage.new(new_file.path || get_real_file(new_file.file))
# => #<FastImage:0x007f9f0a6d5c08 @uri="image_assets/3bb19772-4350-4f96-9279-688740bb7628.png",
@options={:type_only=>false, :timeout=>2, :raise_on_failure=>false, :proxy=>nil, :http_header=>{}},
@property=:size, 
@parsed_uri=#<Addressable::URI:0x3fcf8536ac10 URI:image_assets/3bb19772-4350-4f96-9279-688740bb7628.png>
>
image.type
# => nil

So backing all the way out, if I try at the top level to use

asset = Assets::Image.create( 
  media: original_asset.url, 
  internal_name: "published_#{original_asset.internal_name}" 
)

It does actually save, but with no image:

asset.url
# => nil
asset.media
=> #<ImageUploader:0x007f9f0a67f178 
@model=#<Assets::Image id: 37848, created_at: "2016-05-13 15:45:10", media: nil, type: "Assets::Image", updated_at: "2016-05-13 15:45:10", internal_name: "published_3bb19772-4350-4f96-9279-688740bb7628_378...">,
@mounted_as=:media, 
@storage=#<CarrierWave::Storage::Fog:0x007f9f0a674cf0 
@uploader=#<ImageUploader:0x007f9f0a67f178 ...>>
>

Is this a use case that was not considered before?
Or, at the very least, is there a way to do what we are doing and pass protect_from_image_bomb! ?

Include unsupported type in error message

Could you include the unsupported image type in the error message. It will be helpful for investigating errors of this kind. I see that error pixel_dimensions_error already includes information (pixel dimensions) from the erroneous image, lets add the type to unsupported_image_type's message.
Thank you for this gem!

Video support

Do/will you have a video support?
Awesome gem BTW! Thank you!

Looks like this does not work

Just tested this gem on my nginx + passenger server (Ubuntu 14.04). I am using CarrierWave with MiniMagick.
When I am uploading this (https://www.bamsoftware.com/bzr/deflate/spark.png.bz2
) file (with removed .bz2 extension), the server is freezes until full reboot.
I am also using https://github.com/musaffa/file_validators gem, but I dont think that it can conflict with carrierwave-bombshelter.

I also found the words in FastImage gem desription:
'But take care to sanitise the strings passed to FastImage; it will try to read from whatever is passed.'
Is it safe to send FastImage.size(new_file.path) directrly without any sanitizing?

Possible to remove dependency of `activesupport`?

The only place that is using it is https://github.com/DarthSim/carrierwave-bombshelter/blob/master/lib/carrierwave/bombshelter.rb#L14

But it can be replaced easily with

def self.included(base)
  base.class_eval do # or `module_eval`
      # `before` puts callback in the end of queue, but we need to run this
      # callback first.
      # before :cache, :protect_from_image_bomb!
      self._before_callbacks = _before_callbacks.merge(
        cache: [:protect_from_image_bomb!] + _before_callbacks[:cache]
      )
  end
end