fix: stop emitting the literal string "None" for undecodable text - #2384
fix: stop emitting the literal string "None" for undecodable text#2384hylin (linhongyu510) wants to merge 1 commit into
Conversation
|
@microsoft-github-policy-service agree |
CsvConverter and PlainTextConverter fall back to charset detection when no
charset is declared:
content = str(from_bytes(file_stream.read()).best())
charset_normalizer's best() returns None when detection fails outright, and
str(None) is the three-character string "None". A CSV that cannot be decoded
therefore converts to a one-cell table holding text that was never in the
file, with no error raised:
| None |
| --- |
.txt / .md / .json go through PlainTextConverter and return "None" as the
whole document.
The repo already handles this everywhere else - _markitdown.py guards with
`if charset_result is not None` and OutlookMsgConverter falls back to
`data.decode("utf-8", errors="ignore")`. Apply that same fallback in these
two converters so undecodable bytes degrade lossily instead of inventing
content.
a3c7c40 to
7a99eb7
Compare
|
Rebased onto the latest Verified on the rebased head: For the wider suite I compared against a clean Ready for review when convenient. |
Summary
CsvConverterandPlainTextConvertercan emit the literal three-characterstring
Noneas document content when charset detection fails, with no errorraised.
Both fall back to detection when no charset is declared:
charset_normalizer.from_bytes(...).best()returnsNonewhen it cannot settleon an encoding, and
str(None) == "None".Details
Reproduced on
main(a035350) with bytes that defeat detection —b"\xff\xfe" + bytes([0xD8, 0x00, 0xDC]):.csv| None || --- || \x00 || --- |.txtNone\x00.mdNone\x00.jsonNone\x00The failure mode is what makes this worth fixing: the output is not empty and
not an error, it is a well-formed one-cell Markdown table containing a word
that never appeared in the source. Anything consuming the result downstream
(an index, an LLM prompt) sees
Noneas real document text.This is already handled correctly elsewhere in the repo, so the fix follows
existing precedent rather than inventing a policy:
_markitdown.py:751—if charset_result is not None:_outlook_msg_converter.py:290—if detected is not None:… elsedata.decode("utf-8", errors="ignore")Both converters now use that same lossy-decode fallback. Undecodable bytes
degrade instead of fabricating content, and the declared-charset and
successful-detection paths are untouched.
Scope note:
grep -rn 'str(from_bytes(.*).best())'finds exactly these twocall sites, so this covers all of them.
Related Issues
None found — searched open issues for
None/ charset / undecodable and foundno existing report. Discovered while probing the text converters directly.
How to Validate
Expected:
7 passed.To confirm the tests are load-bearing, stash the two converter changes and
re-run with the new test file in place:
The 4 failures are the undecodable assertions. The 3 that still pass are
deliberate guards: one asserts the fixture really is undecodable (so a future
charset_normalizerthat starts decoding these bytes cannot quietly turn thesuite green), and two cover the Shift-JIS detection and declared-charset paths
to show the fallback does not swallow the normal cases.
Full suite, before and after this change:
The 5 failures are identical in both runs and unrelated to this change — they
are missing optional dependencies in my local env (
azure-ai-contentunderstanding,azure-ai-documentintelligence,speech_recognition,pydub), which I couldnot install because this machine runs Python 3.14 and
youtube-transcript-api~=1.0.0in the
allextra requires<3.14. CI runs 3.10–3.13, where[all]installscleanly, so those 5 should not appear there.
blackreports all three files unchanged.