Skip to content

fix(mhtml): don't decode the HTML part twice when the page has a meta charset - #4388

Open
r0h1tb wants to merge 1 commit into
docling-project:mainfrom
r0h1tb:fix/mhtml-meta-charset
Open

r0h1tb wants to merge 1 commit into
docling-project:mainfrom
r0h1tb:fix/mhtml-meta-charset

Conversation

@r0h1tb

@r0h1tb r0h1tb commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Problem

If an archive's HTML part declares a charset in its MIME header and the page also has its own <meta charset>, non-ASCII text in a single-byte charset comes out garbled:

Content-Type: text/html; charset=windows-1251

<html><head><meta charset="windows-1251"></head><body><p>Привет, мир</p></body></html>
doc.export_to_markdown()  # 'Привет, мир'

_decode_mhtml_html decoded the part with its declared charset and re-encoded it as UTF-8. BeautifulSoup then picked up the <meta> declaration and decoded those UTF-8 bytes again as windows-1251. The docstring already noted this as a known limitation.

Fix

Return the decoded text and give it to the parser as str, so there's no second decode. The part's charset wins over the page's own declaration, the same precedence HTML gives a Content-Type charset over <meta>. Parts without a charset still reach the parser as bytes and get detected from the <meta> tag as before (the Blink fixture is that case).

Tests

Parametrized test_declared_non_utf8_charset_is_decoded over no meta tag, <meta charset> and <meta http-equiv="Content-Type">. Without the fix:

FAILED tests/test_backend_mhtml.py::test_declared_non_utf8_charset_is_decoded[meta-charset]
FAILED tests/test_backend_mhtml.py::test_declared_non_utf8_charset_is_decoded[meta-http-equiv]
AssertionError: assert 'Привет, мир' in 'Привет, РјРёСЂ'

html/mhtml/epub/email/jats backend tests: 173 passed, 7 failed before; 175 passed, 5 failed after. The 5 are remote-image tests that fail on both sides locally because example.com doesn't resolve here. ruff, ty and tach are clean, with no new ty diagnostics.

Checklist:

  • Documentation has been updated, if necessary.
  • Examples have been added, if necessary.
  • Tests have been added, if necessary.

… charset

When the HTML part of an archive declares a charset, it was decoded and
re-encoded as UTF-8 before parsing. BeautifulSoup then followed the page's
own <meta charset> and decoded those UTF-8 bytes a second time, so text in
windows-1252, iso-8859-1 and other single-byte charsets came out as
mojibake.

Hand the decoded text to the parser instead, so the charset declared on
the part is the only one applied.

Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ DCO Check Passed

Thanks @r0h1tb, all your commits are properly signed off. 🎉

@mergify

mergify Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

@codecov

codecov Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@chrikrah chrikrah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@r0h1tb I would merge this. At 81ea2e8 the html and mhtml suites pass, and reverting html_backend.py to the merge base 2d5c590 with your tests kept fails the two meta cases:

$ python -m pytest tests/test_backend_mhtml.py tests/test_backend_html.py -q
84 passed
# html_backend.py at 2d5c590, tests kept
2 failed, 82 passed    # test_declared_non_utf8_charset_is_decoded[meta-charset], [meta-http-equiv]

$ python -m pytest tests/test_backend_email.py tests/test_backend_epub.py -q
34 passed

The fix also covers a case the parametrization does not:

$ python docling-4388_mhtml_probe.py    # at 2d5c590, then at 81ea2e8
# each archive: multipart/related, one base64 text/html part holding <p>Привет</p>,
# converted from a DocumentStream by DocumentConverter(allowed_formats=[InputFormat.MHTML])
MIME part charset / <meta> / body bytes        2d5c590              81ea2e8
utf-8 / windows-1251 / utf-8                    'Привет'         'Привет'
windows-1251 / utf-8 / windows-1251             'Привет'             'Привет'
x-bogus / windows-1251 / windows-1251           'Привет'             'Привет'
none / windows-1251 / windows-1251              'Привет'             'Привет'

non-blocking: the first row, a UTF-8 part whose <meta> still says windows-1251, is not covered. A sibling test with a utf-8 part and that meta would pin it. The branch merges cleanly onto d6f0307.

@ceberam, you reviewed and merged the MHTML backend in #4184; could you take a look at this one?

This branch has not been deployed

No deployments
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