rails / marcel

Find the mime type of files, examining file, filename and declared type
Apache License 2.0
386 stars 67 forks source link

compatibility is broken with certain types of office documents #35

Closed fursich closed 3 years ago

fursich commented 3 years ago

Hi - first of all A HUGE THANKS for your recent hard work! I can't thank you enough for your dedication that saved entire rails community (and of course, myself included) from license problems.

By the way, we have just upgraded our Rails app with marcel 1.0.0, and found that some tests started failing due to mis-detection of xlsx formats.

summary

> Marcel::MimeType.for File.open('spec/fixtures/document/excel/sample.xlsx')
=> "application/zip"

reproduction case

To reproduce above case, let me submit a PR separately with mis-detected files attached as fixture.

quick analysis and a few thoughts..

It looks like the parts that consists of office open document doesn't have a fixed order. So in some cases, [Contents_Type].xml doesn't come immediately after the first 30 bytes.

Currently marcel is very specific about where to expect[Contents_Type].xml, whereas mimemagic used to look up wider ranges to find the part.

Would that make sense to relax the area for magic match, so that it can detect much varied xlsx's?

wilkosz commented 3 years ago

I think there may be a few further inconsistencies with various other file formats.

> Marcel::MimeType.for File.open('...')
=> "application/zip"

Affected file types ott ots odp otp ppt pptm odt ods odg

Example Files at https://drive.google.com/drive/folders/1124-kGCs5sJ1Ng5pXmIzwQLk1CgnWjb4?usp=sharing

rocket-turtle commented 3 years ago

@wilkosz does this change fixes the result for this files? https://github.com/rails/marcel/pull/38

wilkosz commented 3 years ago

@wilkosz does this change fixes the result for this files?

38

Thanks, @rocket-turtle, this solves all our inconsistencies