kreeti / kt-paperclip

Easy file attachment management for ActiveRecord
Other
275 stars 95 forks source link

Make `content_type_mappings` option more user-friendly #43

Closed svyatov closed 1 year ago

svyatov commented 3 years ago

Is your feature request related to a problem? Please describe. I spent quite some time debugging an issue in our project due to misconfiguration of content_type_mappings - keys were strings instead of symbols.

Describe the solution you'd like I'd like for content_type_mappings option to be more user-friendly and/or fool-proof. Two options here:

  1. Raise an error when there are string keys provided;
  2. Don't raise any errors and make this option a "hash with indifferent access".

Additional context This issue was discovered when we tried to upload .json files. Linux file utility returns text/plain content type while content type from extension is application/json. So in order to fix this we had to add a custom mapping:

Paperclip.options[:content_type_mappings] = {
  'json' => 'text/plain'
}

The above code didn't work, because the key is a string, not a symbol. The following code works as expected:

Paperclip.options[:content_type_mappings] = {
  json: 'text/plain'
}

This happens because of this code:

https://github.com/kreeti/kt-paperclip/blob/f0367733ddf37141ae00517c6697cc3dd233dab9/lib/paperclip/media_type_spoof_detector.rb#L86-L88

There was a related issue in the original repo: https://github.com/thoughtbot/paperclip/issues/2425

I'm happy to provide a PR once we settle on the exact solution: raise or accept both strings and symbols. Plus we could fix the case-sensitivity issue mentioned above as well.

ssinghi commented 3 years ago

Thank you @svyatov! I think the code should support both strings and symbols, please create a PR for this. Thanks!