Repository navigation
Conversation
|
Alex0AI please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Community review (not a maintainer, does not clear the merge gate). I read the full diff (the one-line change in __main__.py and the new tests in tests/test_cli_misc.py), then read how mime_type_hint flows into StreamInfo and _markitdown.py. I ran it on Windows with Python 3.13.
What I ran:
- tests/test_cli_misc.py on this PR head: 25 passed. With only
__main__.pyput back to origin/main, 3 fail: the twotest_mime_type_parameters_are_preservedcases that contain a slash inside a parameter, andtest_invalid_media_type_is_not_hidden_by_parameters[text; profile=a/b]. The other new parameters pass on main too (as expected, since they already had the right slash count). - The real CLI with stdin input (
python -m markitdown --mime-type ...), not the mocked path:text/html,text/html; charset=utf-8,text/html; profile="https://example.org/p"and upper-caseTEXT/HTML; profile="https://e/p"all produced the same# Hiheading.text/csv; profile=...produced the table andapplication/json; profile=...passed the JSON through. On main the profile form exits with "Invalid MIME type: text/html; profile="https://example.org/p\"".text; profile=a/bis still rejected on the PR, because the check now looks only at the part before the first;.
The reasoning holds: the number of slashes in the type itself is what matters, and the old count("/") != 1 over the whole string rejected valid parameter values such as a quoted profile URI.
Notes, none blocking:
- The full string, parameters included, is still stored in
StreamInfo.mimetype(as it already was for parameter hints without a slash, liketext/html; charset=utf-8). I read_markitdown.pylines 745 to 790: the extension guess viamimetypes.guess_all_extensions(base_guess.mimetype)will find nothing for a hint with parameters, and the comparison with the Magika result is an exact string compare. In my CLI runs the converters still matched, because they compare by prefix, but I only tried HTML, CSV and JSON, not binary formats such as PDF or DOCX. Splitting the parameters off intocharsetand the bare type, as theContent-Typeheader handling does around line 538 to 544, would make this uniform, but that is a separate change. - The check is a plain split on the first
;, so a hint such astext/plain;a="b;c/d"is accepted (I ran it, outputx). That is fine, since the type before the first;is still validated.
Looks correct and small.
Summary
The CLI currently counts every slash in
--mime-type, including slashes in parameters. A valid hint such asapplication/json; profile="https://example.org/a/b"exits withInvalid MIME typebefore conversion. Conversely, a missing type/subtype separator can be masked by a slash in a parameter (text; profile=a/b).Check the existing separator requirement only on the media type before the first semicolon. Preserve the entire original hint in StreamInfo so converters/plugins still receive its parameters. This keeps the fix scoped to the CLI's existing validation rather than introducing a new MIME parser.
Tests
git diff --checkpassed.Investigated and implemented with OpenAI Codex assistance.