Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional auto content type detection #2465

Closed
alexpooley opened this issue Mar 31, 2020 · 1 comment
Closed

Optional auto content type detection #2465

alexpooley opened this issue Mar 31, 2020 · 1 comment

Comments

@alexpooley
Copy link

Parsing file headers for content type is inefficient and can be problematic on certain file systems. For large systems the inefficiency can add up.

I've worked through the code and it does not seem necessary to automatically detect content type on each upload. Is content type needed as part of the core upload process? It seems to me only important if we are filtering by type, such as content_type_whitelist.

Currently calls to SanitizedFile#content_type will trigger a parse of the file headers to determine file type. On a default upload, the specific pathway is through the move_to and copy_to methods.

I want to create a PR to make auto content type detection optional.

This would be through the standard Uploader::Configuration mechanisms. The problem with this is that I need to pull content type detection out of SanitizedFile and in to the Uploader itself, because otherwise SanitizedFile does not have access to the configuration.

This is a substantial change so I wanted to throw it out there before I proceeded. Does the community have any feedback on this?

@mshibuya
Copy link
Member

This was introduced as a mitigation to ImageTragick in #1934.
I guess Content-type spoofing prevention will be somewhat important, and also ActiveStorage uses similar auto-detection.
But some sort of efficiency improvement may be possible...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants