Paperclip media spoof check is ignoring the results from the file command

[cnk]

Most of the issues I see listed here are about Paperclip 4.2 preventing upload and saving of files based on the output of file but I am having the opposite problem. I want the media spoofing check feature. But even though I see log messages about paperclip calling file, it isn't doing anything with the result.

I have created a minimal Rails + Paperclip project to demonstrate what I am seeing: https://github.com/cnk/paperclip-issue I have included a faked file - a PDF with the file extension changed to .doc. My browser sends content type information based on the extension and the file gets saved with a recorded content type of application/msword even though file correctly identifies the saved file's mime type as application/pdf.

If I lock down the content type requirements in my model, paperclip restricts upload based on the browser-reported content type and NOT the correct content type reported by file. (I see the same results with file 5.18 on a Mac and file 4.17 on RHEL 5.

(And one other oddity - file gets called twice even though this model has no additional styles.)

[cnk]

After more poking around, I found that the Paperclip::FileAdapter hardcodes getting it's content type information from ContentTypeDetector but Paperclip::UploadedFileAdapter appears to be expecting you to configure what detector to use. Creating an initializer with the line:

Paperclip::UploadedFileAdapter.content_type_detector = Paperclip::FileCommandContentTypeDetector

fixes my primary complaint - that Paperclip was just accepting whatever content type came on with the request parameters. However it creates a secondary issue: with the content_type now correctly set in the model, the media type spoof detection doesn't fire because the content type matches (since both come from the file command).

I would like to fix this in a way that can get pulled back into Paperclip but need some guidance on the desired behavior for the main project. Questions:

1. It looks like at least some of the IO adapters want to make content_type detection a pluggable module. Should we refactor all adapters to use a default detector which can be overriden by a configuration - in the way that the filename_cleaner currently works? That seems like a great way to provide a sensible default that can be altered at will.
2. What do people want the default behavior to be? Use whatever content type we are told as the type - and then use the media type spoof detection to validate that? That seems the most reasonable default.
3. Why do I see multiple temp files (and multiple calls to file to check their type) for an upload with no additional styles? There is a related issue already open, Paperclip copies too many files to the file system #1642, but with no comments. The tone was rude but the observations might be valid

[kevinmtrowbridge]

I went on an amazing adventure today and ended up here. Thanks for your work, @cnk.

With two different Rails apps (my own, and cnk's paperclip-issuelinked to above), I found the following behavior, which seems strange:

Stepping through this code in uploaded_file_adapter.rb ...

    def determine_content_type
      content_type = @target.content_type.to_s.strip
      if content_type_detector
        content_type = content_type_detector.new(@target.path).detect
      end
      content_type
    end

content_type = @target.content_type.to_s.strip: ActionDispatch::Http::UploadedFile's content_type (which, seems to be the MIME type as determined by the browser, from the file extension).

if content_type_detector checks for the presence of content_type_detector ... by default, this seems to be nil. Note, this is even if you have configured a validates_with content type in the model, as discussed in the Readme.

This is with Rails 4.0.13 and Rails 4.2.1, and with Paperclip 4.2.0 and 4.2.1.

If I specifically configure with an initializer (as cnk pointed out), then content_type_detector is not nil, and it invokes the sophisticated ContentTypeDetector and discovers the true ContentType.

config/initializers/paperclip_content_type_detection.rb

Paperclip::UploadedFileAdapter.content_type_detector = Paperclip::ContentTypeDetector

I guess I still suspect both cnk and I are somehow misconfiguring, because I see others with bugs that seem to stem from ContentTypeDetector running normally: #1713, #1479

I'd love to delete that initializer. What are @cnk and I doing wrong?

[ivanoblomov]

Paperclip::UploadedFileAdapter.content_type_detector = Paperclip::ContentTypeDetector should be the default.

Without it, Paperclip allows the upload of anything, including executables, if the client spoofs the attachment's content-type to the permitted values.

[ivanoblomov]

Opened #2270 to default to a safer setting.

[tute]

Merged the fix in #2270. Thank you all very much.