Bug before :cache callback returns sanitized file which loses unsanitized original filename

[Neil-Ni]

Hi guys,

Just came across an unexpected behavior in before :cache callback today, and would like to raise an issue. From official wiki, there's a snippet on how to save the original filename:

  before :cache, :save_original_filename
  def save_original_filename(file)
    model.original_filename ||= file.original_filename if file.respond_to?(:original_filename)
  end

I was expecting the callback to return an original file, but after spending some time debugging I've found that before :cache callback doesn't actually execute before the cache. It actually allows Uploader::Cache.cache! to be called with a new_file and then executing callbacks afterwards with an sanitized file.

Not sure if this is by design but here are the related links to the code:

related code: https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/uploader/cache.rb#L118 https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/uploader/cache.rb#L137-L139 https://github.com/carrierwaveuploader/carrierwave/blob/master/lib/carrierwave/uploader/callbacks.rb#L13-L17

here's the spec I've written to clarify this error:

https://github.com/Neil-Ni/carrierwave/blob/f17ee898cf76866ada74fa0ee9f7170ce8f7c5d1/spec/uploader/callback_spec.rb#L29-L46

describe CarrierWave::Uploader do
 ...
  context 'when before cache callback exists' do
    before do
      @uploader_class = Class.new(CarrierWave::Uploader::Base)
      @uploader = @uploader_class.new
      @file = File.open(file_path('test.jpg'))
      @sanitized_file = CarrierWave::SanitizedFile.new(@file)
      allow(@sanitized_file).to receive(:original_filename).at_least(:once).and_return("test-s,%&m#st?.jpg")
      allow(@uploader).to receive(:before_cache_callback)
      @uploader_class.before :cache, :before_cache_callback
      @uploader.cache!(@sanitized_file)
    end

    it "returns original file with unsanitized filename" do
      expect(@uploader).to have_received(:before_cache_callback) do |arg|
        expect(arg.original_filename).to eq "test-s,%&m#st?.jpg"
      end
    end
  end
end

Rspec Error:

1) CarrierWave::Uploader when before cache callback exists returns original file with unsanitize filename
     Failure/Error: expect(arg.original_filename).to eq "test-s,%&m#st?.jpg"

       expected: "test-s,%&m#st?.jpg"
            got: "test-s___m_st_.jpg"

       (compared using ==)
     # ./spec/uploader/callback_spec.rb:43:in `block (4 levels) in <top (required)>'
     # ./spec/uploader/callback_spec.rb:42:in `block (3 levels) in <top (required)>'

[thomasfedb]

Thanks for the report @Neil-Ni.

[SergeyKishenin]

Any updates or workaround for this? @thomasfedb @Neil-Ni

[sarojkh]

@SergeyKishenin Did you ever get to a workaround?

[ajoanny]

There is a work around you could add this to your uploader :

def cache! file
  save_original_filename file
  super
end

[sarojkh]

@ajoanny Thanks a lot. It works!

[blainelawson]

There is a work around you could add this to your uploader :

def cache! file
  save_original_filename file
  super
end

Is save_original_filename deprecated? I'm getting a NoMethodError.

[sarojkh]

@blainelawson The method save_original_filename doesn't exist in carrierwave. If you look carefully at the issue description above, it is a custom method used to get around the issue.

[blainelawson]

@sarojkh You are correct. My bad.