carrierwaveuploader / carrierwave

Classier solution for file uploads for Rails, Sinatra and other Ruby web frameworks
https://github.com/carrierwaveuploader/carrierwave
8.78k stars 1.66k forks source link

Errno::ENAMETOOLONG when downloading files with long names #2042

Closed fschwahn closed 8 years ago

fschwahn commented 8 years ago

Hi, when downloading a file (using remote_file_url=) with a very long filename Errno::ENAMETOOLONG is raised. This is probably dependent on the file system (in my case it happened on heroku, couldn't find info about which file system they are using), but it may be a good idea to sanitize these filenames?

Example: https://www.welt.de/img/motor/news/mobile158642604/4971350517-ci16x9-w1200/Der-X6-behaelt-weiterhin-sein-fuer-europaeische-Verhaeltnisse-rekordverdaechtiges-Format-blaest-sich-ungehemmt-auf-deutlich-mehr-als-zwei-spiegeltoetende-Meter-Breite-zeigt-sich-in-der-Frontpartie-jedoch-stimmiger-und-naeher-am-gefaelligeren-Basisbruder-X5.jpg

mshibuya commented 8 years ago

Filename can be customized by uploader's filename method, name it anything you want.

fschwahn commented 8 years ago

Sorry, should have included the stack trace:

carrierwave (0.11.2) lib/carrierwave/sanitized_file.rb:213:in `copy_to'
carrierwave (0.11.2) lib/carrierwave/uploader/cache.rb:143:in `block in cache!'
carrierwave (0.11.2) lib/carrierwave/uploader/callbacks.rb:17:in `with_callbacks'
carrierwave (0.11.2) lib/carrierwave/uploader/cache.rb:134:in `cache!'
carrierwave (0.11.2) lib/carrierwave/uploader/download.rb:72:in `download!'

The error happens during caching, the filename method has no effect there. I just tested it, the error remains when setting the filename in the uploader.

What does work however is to overwrite cache_name to truncate filenames which are too long. But this feels like reaching in to the internals of carrierwave - I doesn't feel exactly right.

What I ended up doing is monkeypatching the original_filename-method of CarrierWave::Uploader::Download::RemoteFile to truncate long filenames.

I can make add a pull request if you think this is worthwhile? I'm not sure whether or not this is out of scope for this project.

obliviusm commented 7 years ago

having exactly same issue

ishields commented 3 years ago

Hello, was this ever resolved? I'm having this exact issue. @fschwahn did you ever make a PR for this / do you mind sharing?

@mshibuya any thoughts on this? This doesn't seem like something you can override without monkey patching as @fschwahn mentioned.

ishields commented 3 years ago

Here is the monkey patch file I wrote which fixed the issue. I agree that this should be integrated into the actual gem as it is likely effecting everyone (but is an edge case). Perhaps it makes sense to have a "max_remote_image_url_length" config or something to that effect.?

# This file moneky patches this method to prevent an error uploading with remote_image_url
# If the url used is too long we were getting an error Errno::ENAMETOOLONG
# https://github.com/carrierwaveuploader/carrierwave/issues/2042
module CarrierWave
  module Downloader
    class RemoteFile
      def original_filename
        filename = filename_from_header || filename_from_uri
        mime_type = MiniMime.lookup_by_content_type(file.content_type)
        unless File.extname(filename).present? || mime_type.blank?
          filename = "#{filename}.#{mime_type.extension}"
        end

        if filename&.length.to_i > 100
          filename = "#{filename[0..5]}-#{SecureRandom.hex}"
        end

        filename
      end
    end
  end
end