Skip to content

Fix UnicodeDecodeError when guessed charset doesn't hold for the whole file - #2346

Open
Oscar A Garcia (alexandregfonseca) wants to merge 4 commits into
microsoft:mainfrom
alexandregfonseca:fix/plaintext-charset-fallback
Open

Fix UnicodeDecodeError when guessed charset doesn't hold for the whole file#2346
Oscar A Garcia (alexandregfonseca) wants to merge 4 commits into
microsoft:mainfrom
alexandregfonseca:fix/plaintext-charset-fallback

Conversation

@alexandregfonseca

Copy link
Copy Markdown

Summary

  • PlainTextConverter guesses the file's charset from only the first 4096 bytes of the stream (_markitdown.py). If that prefix happens to be pure ASCII but a multi-byte UTF-8 character appears later in the file, the charset is mis-detected as ascii, and the subsequent full-content .decode("ascii") raises UnicodeDecodeError, causing conversion to fail entirely.
  • This is easy to hit in practice: running markitdown README.md on this very repo's README fails, because the README is ASCII for its first 4096 bytes and contains an em dash () shortly after.
  • Fix: when decoding with the guessed charset fails, fall back to detecting the charset from the full file content instead of just the first 4096 bytes.

Test plan

  • Added a regression test (test_plain_text_charset_guessed_from_prefix) reproducing the exact scenario: 4096 bytes of ASCII followed by a UTF-8 multi-byte character.
  • pytest packages/markitdown/tests/test_module_misc.py passes (one unrelated pre-existing failure in test_speech_transcription due to ffprobe not being installed in my environment).
  • Manually verified markitdown README.md now succeeds instead of raising FileConversionException.

PlainTextConverter guesses the charset from only the first 4096 bytes of
the stream. For files that are pure ASCII in that window but contain
multi-byte UTF-8 characters later on (e.g. an em dash), the charset is
guessed as "ascii" and the subsequent full-content decode raises
UnicodeDecodeError, causing conversion to fail entirely.

Fall back to detecting the charset from the full content when decoding
with the guessed charset fails.
Covers the case where the guessed charset (based on the first 4096
bytes) is ASCII but the file contains multi-byte UTF-8 characters later
on.
@alexandregfonseca

Oscar A Garcia (alexandregfonseca) commented Aug 27, 2026 via email

Copy link
Copy Markdown
Author

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Community review, not a merge gate.

The fix in packages/markitdown/src/markitdown/converters/_plain_text_converter.py wraps the stream_info.charset-guided decode in try/except UnicodeDecodeError, falling back to from_bytes(raw_content).best() full-content detection on failure, same fallback already used when no charset guess exists at all. This is a real defensive improvement: stream_info.charset can come from more than one source that isn't guaranteed to hold for the whole file, including an HTTP Content-Type: charset=... header (I checked _markitdown.py and this header value is used directly as a charset guess with no validation against the actual body).

One thing I want to flag rather than just approve past: the PR's code comment and test docstring both say the charset is "guessed from only the first 4096 bytes," but I checked _get_stream_info_guesses in _markitdown.py and the actual magika-based charset sample is file_stream.read(65536) (64KB), not 4096 bytes. The new test's input is "a" * 4096 + "em dash — end", which at roughly 4110 bytes total is entirely inside that 64KB sample window, so it isn't actually testing the "sample didn't cover the whole file" mechanism the comment describes. It may still reproduce a real UnicodeDecodeError for a different reason (charset_normalizer's confidence scoring on content that's over 99% ASCII could still report ascii as best even having seen the em dash in-sample), but I did not verify that charset_normalizer actually returns "ascii" for this specific byte sequence, so I can't confirm the test exercises the bug through the mechanism it claims to.

The fix itself is sound and safe regardless of which exact mechanism triggers the mismatch. The discrepancy is in the stated reasoning and test rationale, not in whether the try/except is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants