dwilkie / carrierwave_direct

Process your uploads in the background by uploading directly to S3
MIT License
839 stars 181 forks source link

FilenameFormatValidator fails for every new object #227

Open pramodshinde opened 5 years ago

pramodshinde commented 5 years ago

I am using carrierwave_direct for direct uploads and server side uploads. For every new object I am getting following error

"avatar: is invalid. "

f = File.open("/Users/pramod/Downloads/vt3.pdf") 
user = User.new(avatar: f)
user.valid? # false

I dived into gems code found that following condition in failing for each new object as validations are getting invoked on :create and this conditions is always turns to be true.

record.send("has_#{attribute}_upload?") && record.send("#{attribute}_key") !~ record.send(attribute).key_regexp

Here record.send("#{attribute}_key") is "public/user/avatar//" record.send(attribute).key_regexp is /\A(public\/user\/avatar\/|uploads\/tmp)\/[a-f\d\-]+\/.+\.(?i)\w+(?-i)\z/

I thought "public/user/avatar//" invalid and this condition getting true because of this path but when I tried putting valid path like "public/user/avatar/199164/sample.pdf" !~ `/\A(public\/user\/avatar\/|uploads\/tmp)\/[a-f\d-]+\/.+.(?i)\w+(?-i)\z/``

This is also failing, Not sure whats wrong? Is this regex checking wrong or path which I am generating is wrong?

Model:

class User < ActiveRecord::Base
  ...
  mount_uploader :avatar, AvatarUploader
  ...
end
....

Uploader 
```ruby
class AvatarUploader < CarrierWave::Uploader::Base
  include CarrierWave::MiniMagick
  include CarrierWaveDirect::Uploader
  include CarrierWave::Processing::MiniMagick

  def store_dir
    "public/#{model.class.name.underscore}/#{mounted_as}/#{model.id}"
  end
end

Carrierwave Config

CarrierWave.configure do |config|
  config.storage = :fog
  config.fog_credentials = {
    provider: 'AWS',
    aws_access_key_id: ENV['AWS_ACCESS_KEY_ID'],
    aws_secret_access_key: ENV['AWS_SECRET_ACCESS_KEY'],
    aws_signature_version: 4,
    region: ENV['AWS_REGION']
  }

  config.will_include_content_type = true
  config.use_action_status = true
  config.asset_host                       = ENV['CDN_HOST']
  config.fog_directory                    = ENV['S3_BUCKET']
  config.fog_public                       = false
  config.fog_authenticated_url_expiration = 900
  config.fog_attributes                   = { 'Cache-Control' => 'max-age = 315576000' }
  config.max_file_size                    = 500.megabytes
end
abepark01 commented 5 years ago

@pramodshinde I am getting the same issue.

This works fine when I upload an image for an existing model... but fails when a new model is created.

in-hale commented 5 years ago

@pramodshinde I am getting the same issue too when I'm creating an upload from a remote location. It can be solved by config.validate_filename_format = false in your carrierwave.rb though, but I'm not sure it is the best option. The gem omits the unique part of the store_dir in this case, which causes files to be stored by the same location ("uploads/marketplace/photo/image/" in my case).

pramodshinde commented 5 years ago

@in-hale but adding config.validate_filename_format = false seems work around, Is the following condition is true for you in every case?

record.send("has_#{attribute}_upload?") && record.send("#{attribute}_key") !~ record.send(attribute).key_regexp
p8 commented 5 years ago

Carrierwave-direct doesn't currently support creating an upload be setting the file on the model. It expects the attribute_key to be set and then it downloads the file from S3.

For tests you can use sample_key in https://github.com/dwilkie/carrierwave_direct/blob/32fb37708671f72fb7fc6c1a0c7196e5f8291a2d/lib/carrierwave_direct/test/helpers.rb But that might be useful in other situations like above as well.

pramodshinde commented 5 years ago

Not sure @p8 What you are suggesting here? Can you elaborate for my use case?

in-hale commented 5 years ago

@pramodshinde It's not, and it's exactly the line where things go wrong. However, I found a better walkaround here: You can add something like this to the model you mounting your uploader to This is gonna take care of your key in this case

before_validation :ensure_proper_key_is_set

private

# 'image' here is the 'mounted_as' value for the uploader
def ensure_proper_key_is_set
  return unless image_key == image.store_dir + "/"

  self.image_key = "#{SecureRandom.uuid}/${filename}"
end

And then you can set this back to true: config.validate_filename_format = true