fix(outlook): read .msg string properties saved in the non-Unicode format - #2295
Merged
Conversation
Every string property in a .msg lives under a stream whose name ends in its MAPI type: 001F for PT_UNICODE (UTF-16LE) or 001E for PT_STRING8, written in the message's code page. Outlook writes one or the other for a given message, never both, so a message saved in the legacy non-Unicode format carries no 001F streams at all. The converter addressed only the 001F names. Such a message therefore came out as bare scaffolding -- "# Email Message" followed by "## Content" -- with From, To, Subject and the body all silently dropped, and no error raised. Each property is now read from the 001F stream and, failing that, from its 001E counterpart. PT_STRING8 streams record no encoding of their own, so the charset is detected with charset_normalizer, as is already done for other 8-bit sources in the codebase. The Unicode path is unchanged.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
ANSI streams must use the message’s declared code page to avoid corrupting valid non-Western text.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds support for legacy non-Unicode Outlook .msg string properties.
Changes:
- Falls back from Unicode (
001F) to ANSI (001E) property streams. - Adds ANSI and Unicode regression tests.
File summaries
| File | Description |
|---|---|
_outlook_msg_converter.py |
Reads and decodes ANSI MAPI properties. |
test_outlook_msg_ansi.py |
Tests ANSI and Unicode message conversion. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
ANSI terminators and several code-page paths can currently corrupt converted text.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
packages/markitdown/src/markitdown/converters/_outlook_msg_converter.py:160
PR_INTERNET_CPIDonly declares the encoding ofPR_BODY/PR_BODY_HTML, so using it whenPR_MESSAGE_CODEPAGEis absent can silently decode ANSI From/To/Subject bytes with the body's unrelated codec (single-byte codecs usually will not raise). Leave header encoding unset in that case so the existing detector handles the undeclared header encoding.
header_encoding = message_encoding or internet_encoding
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+65
to
+66
| 50221: "iso2022_jp", | ||
| 50222: "iso2022_jp", |
Comment on lines
+280
to
+292
| if not data: | ||
| return None | ||
|
|
||
| if encoding is not None: | ||
| try: | ||
| return data.decode(encoding).strip() | ||
| except (UnicodeDecodeError, LookupError): | ||
| pass # The declared code page does not fit; fall back to detection | ||
|
|
||
| detected = from_bytes(data).best() | ||
| if detected is not None: | ||
| return str(detected).strip() | ||
| return data.decode("utf-8", errors="ignore").strip() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A
.msgsaved in the legacy non-Unicode format converts to nothing but scaffolding —# Email Messagefollowed by## Content, with From, To, Subject and the body all missing. No exception is raised, so the loss is silent.Why
Every string property in a
.msgis stored under a stream whose name ends in the property's MAPI type:001FPT_UNICODE001EPT_STRING8Outlook writes one or the other for a given message, never both. The converter addressed only the
001Fnames:so for a non-Unicode message every lookup misses,
_get_stream_datareturnsNonefor each, and the headers are skipped by theif value:guard while the body is skipped byif body:.Note this is not reachable by fixing the decode:
_get_stream_datanever sees any bytes, because the streams it names do not exist in the file.The change
Each property is now looked up by its tag, trying
001Ffirst and falling back to001E:PT_STRING8streams record no encoding of their own, so the charset is detected withcharset_normalizer, consistent with how other 8-bit sources are handled in the codebase. ReadingPR_INTERNET_CPIDout of__properties_version1.0would give the declared code page instead, but that means parsing the fixed-property stream by hand, and detection is what the rest of the converters already rely on. Happy to switch if you would rather have the declared value.The Unicode path is untouched — it is still tried first, and still decoded exactly as before.
Tests
packages/markitdown/tests/test_outlook_msg_ansi.py. A.msgis an OLE2 compound file and nothing in the dependency set can write that container, so the streams are served through a stand-in forolefile.OleFileIOrather than a binary fixture:test_outlook_msg.msgstill converts, read through realolefile.The two non-Unicode tests fail on
mainand all four pass with this change. The rest of the suite is unaffected andblackreports no changes.Note
This does not overlap #2245, which resolves the Exchange sender DN and the HTML-only body. That PR reads the same
001Fstream names, so a non-Unicode message stays empty there.