Repository navigation
Conversation
|
sean0407 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”),
|
PRABHU KIRAN VANDRANKI (VANDRANKI)
left a comment
There was a problem hiding this comment.
Community review, not a maintainer review and it does not clear the merge gate. I read the full diff and ran it on Windows with Python 3.13.3, against main at 4cc9fa1.
What I ran:
- Called
parse_data_uridirectly on main and on this branch. On main,data:text/plain,Hello#sectionreturnsb'Hello#section', anddata:text/plain;base64,SGVsbG8h#AAAAreturnsb'Hello!\x00\x00\x00', becausebase64.b64decodesilently drops the#and then decodes theAAAA. On the PR the same inputs returnb'Hello'andb'Hello!'. - The new
test_data_uri_fragmentscases: with the PR's test file on main, 6 of the 12 parametrized cases fail (the three fragment inputs, each throughparse_data_uriand throughconvert_uri). On the PR branch all pass. tests/test_module_misc.pyandtests/test_cli_misc.pyon the PR: 201 passed, 1 skipped, 1 failed. The failure istest_speech_transcription(AudioConverter raises FileNotFoundError here). I did not run it on main, and I did not run the full suite for this PR.
The fix is correct as far as I can tell. A # that is not percent-encoded starts a fragment, and the fragment is not part of the payload (RFC 3986, and the Fetch spec's data: URL processing excludes it too; I am relying on my reading of those, not on a test). The %23 case still decodes to #, which your test covers.
Behavior changes worth knowing about, none of them a blocker in my view:
- Hand-written data URIs with a raw
#inside the payload are now truncated at the#. For exampledata:text/html,<p style="color:#fff">x</p>goes from the full string on main to<p style="color:on the PR. Browsers do the same, but anyone who relied on the old lenient behavior will see silent truncation. - A
#in the header part, such asdata:text/plain#frag,Helloordata:#x,Hello, used to parse (with a mimetype oftext/plain#fragand#x). It now raises "Malformed data URI, missing ',' separator". That matches the URL-standard behavior, but it is a new error for inputs that used to work.
Only parse_data_uri is changed, and in this repo its only caller is convert_uri (_markitdown.py line 497), so I do not see other code paths affected.
Summary
#and literal query characters as payload data.parse_data_uriandMarkItDown.convert_uri.Fixes #2690
Verification
pytest -q packages/markitdown/tests/test_module_misc.py -k data_uri_fragments(12 passed)git diff --check