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

Fix some MP3 files not being recognized as such #12809

Closed

Conversation

ClearlyClaire
Copy link
Contributor

Work around libmagic mistakenly identifying some obscure font format for some
files, by not stopping on the first guess, and taking the first that is not
that file format.

Fixes #12693

@koyuawsmbrtn
Copy link
Contributor

Big issue for me. Would help a lot to get it merged before 3.1

cc @Gargron

@saper
Copy link
Contributor

saper commented Mar 13, 2020

Is there an upstream bug? It is nice to have a temporary workaround but this is certainly not the right solution here (big thanks @ThibG for getting it sorted out).

@@ -63,7 +63,12 @@ def appropriate_extension(attachment)
end

def calculated_content_type(attachment)
content_type = Paperclip.run('file', '-b --mime :file', file: attachment.queued_for_write[:original].path).split(/[:;\s]+/).first.chomp
content_types = Paperclip.run('file', '-b -k -r --mime :file', file: attachment.queued_for_write[:original].path).split("\n- ")
content_types -= ['application/x-font-gdos']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would feel better if you aligned the = and put the mime type into a constant variable with a self-explaining name like BOGUS_MIME_TYPES or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed this just in case, but I think this PR isn't needed anymore, as the underlying bug in file has been fixed along with security fixes a while ago.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, in that case the PR can be closed. The workaround is still concerning to me because what if someone tried to upload a real font file, it would then be accepted and treated as MP3...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably wouldn't, as the font file would probably not be detected as mp3 (the bug happens because both the mp3 magic and the font thing are detected, and the font thing used to take precedence even though it was unreliable and uses of that font format are incredibly rare).
Closing.

Work around libmagic mistakenly identifying some obscure font format for some
files, by not stopping on the first guess, and taking the first that is *not*
that file format.

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

Successfully merging this pull request may close these issues.

Error on uploading audio with embedded image
4 participants