fix(file): fix OLE-based file-type auto-detection #3437
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes mis-identification of DOC, PPT, XLS, and MSG files.
scanny
commented
Jul 24, 2024
Comment on lines
+640
to
+672
| # ================================================================================================ | ||
| # Describe `is_json_processable()` | ||
| # ================================================================================================ | ||
|
|
||
|
|
||
| def it_affirms_JSON_is_array_of_objects_from_a_file_path(): | ||
| assert is_json_processable(example_doc_path("simple.json")) is True | ||
|
|
||
|
|
||
| def and_it_affirms_JSON_is_NOT_an_array_of_objects_from_a_file_path(): | ||
| assert is_json_processable(example_doc_path("not-unstructured-payload.json")) is False | ||
|
|
||
|
|
||
| def it_affirms_JSON_is_array_of_objects_from_a_file_like_object_open_for_reading_bytes(): | ||
| with open(example_doc_path("simple.json"), "rb") as f: | ||
| assert is_json_processable(file=f) is True | ||
|
|
||
|
|
||
| def and_it_affirms_JSON_is_NOT_an_array_of_objects_from_a_file_like_object_open_for_reading_bytes(): | ||
| with open(example_doc_path("not-unstructured-payload.json"), "rb") as f: | ||
| assert is_json_processable(file=f) is False | ||
|
|
||
|
|
||
| def it_affirms_JSON_is_array_of_objects_from_text(): | ||
| with open(example_doc_path("simple.json")) as f: | ||
| text = f.read() | ||
| assert is_json_processable(file_text=text) is True | ||
|
|
||
|
|
||
| def and_it_affirms_JSON_is_NOT_an_array_of_objects_from_text(): | ||
| with open(example_doc_path("not-unstructured-payload.json")) as f: | ||
| text = f.read() | ||
| assert is_json_processable(file_text=text) is False |
There was a problem hiding this comment.
These new tests have nothing to do with this fix per se, there were just no tests for this interface function in the test suite.
scanny
commented
Jul 24, 2024
Comment on lines
+117
to
+119
| file_text = _FileTypeDetectionContext.new( | ||
| file_path=filename, file=file, encoding=encoding | ||
| ).text_head |
There was a problem hiding this comment.
Same with this change subsuming _read_file_start_for_file_check(), this just got left out of the prior refactoring PR.
Coniferish
reviewed
Jul 24, 2024
| @@ -351,19 +358,19 @@ def mime_type(self) -> str | None: | |||
|
|
|||
| A `str` return value is always in lower-case. | |||
| """ | |||
| file_path = self.file_path | |||
There was a problem hiding this comment.
why declare this instead of using self.file_path down below?
There was a problem hiding this comment.
Just efficiency, a little more compact too. It's referred to four times in the function. I think more the latter in this case since only two of those dereferencings will happen for any given file.
scanny
force-pushed
the
scanny/fix-ole-file-autodetect
branch
from
206a8f0 to
3827678
Compare
Needs tests, consider reusing `_FileTypeDetectionContext`.text_head.
scanny
force-pushed
the
scanny/fix-ole-file-autodetect
branch
from
3827678 to
3f87de3
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
A DOC, PPT, or XLS file sent to partition() as a file-like object is misidentified as a MSG file and raises an exception in python-oxmsg (which is used to process MSG files).
Fix
DOC, PPT, XLS, and MSG are all Microsoft OLE-based files, aka. Compound File Binary Format (CFBF). These can be reliably distinguished by inspecting magic bytes in certain locations.
libmagicis unreliable at this or doesn't try, reporting the generic"application/x-ole-storage"which corresponds to the "container" CFBF format (vaguely like a Microsoft Zip format) that all these document types are stored in.Unconditionally use
filetype.guess_mime()provided by thefiletypepackage that is part of the base unstructured install. Unlikelibmagic, this package reliably detects the distinguished MIME-type (e.g."application/msword") for OLE file subtypes.Fixes #3364