Fix IMAP mailMove crash on malformed MIME-encoded thread titles #156
Conversation
Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
…headers Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
…rors Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash in IMAP operations when thread titles contain malformed MIME-encoded strings from email subjects. The issue occurred when these encoded strings were used as IMAP folder names, causing imap_mail_move() to fail with "Invalid quoted-printable sequence" errors.
Changes:
- Added MIME header decoding in
ThreadFolderManagerbefore generating folder names - Enhanced error handling in
ImapWrapper.utf8()to gracefully handle malformed MIME strings - Added comprehensive test coverage for both valid and malformed MIME-encoded strings
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| organizer/src/class/ThreadFolderManager.php | Decodes MIME-encoded thread titles before generating IMAP folder names, preventing malformed strings from being passed to IMAP operations |
| organizer/src/class/Imap/ImapWrapper.php | Adds defensive error handling to utf8() method with fallback to mb_decode_mimeheader() when imap_utf8() fails |
| organizer/src/tests/ThreadFolderManagerTest.php | Adds tests for both malformed and valid MIME-encoded thread titles to ensure proper decoding and sanitization |
| organizer/src/tests/ImapWrapperUtf8Test.php | Adds comprehensive test coverage for utf8() method including normal text, valid/malformed MIME strings, and special characters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $title = $entity_id . ' - ' . $thread->title; | ||
|
|
||
| // Decode any MIME-encoded headers (e.g., =?UTF-8?B?...?= or =?iso-8859-1?Q?...?=) | ||
| // Only decode if the string contains MIME encoding markers to avoid corrupting UTF-8 text | ||
| if (strpos($title, '=?') !== false) { | ||
| $title = mb_decode_mimeheader($title); | ||
| } |
There was a problem hiding this comment.
The MIME decoding happens on the combined string $entity_id . ' - ' . $thread->title. If the entity_id contains the marker =?, it would trigger unnecessary MIME decoding. While entity_ids are unlikely to contain such markers in practice, consider checking for MIME markers specifically in $thread->title before combining, or document this assumption to prevent future issues.
| $title = $entity_id . ' - ' . $thread->title; | |
| // Decode any MIME-encoded headers (e.g., =?UTF-8?B?...?= or =?iso-8859-1?Q?...?=) | |
| // Only decode if the string contains MIME encoding markers to avoid corrupting UTF-8 text | |
| if (strpos($title, '=?') !== false) { | |
| $title = mb_decode_mimeheader($title); | |
| } | |
| // Decode MIME-encoded thread title (if any) before combining with entity_id | |
| $decodedTitle = $thread->title; | |
| // Decode any MIME-encoded headers (e.g., =?UTF-8?B?...?= or =?iso-8859-1?Q?...?=) | |
| // Only decode if the string contains MIME encoding markers to avoid corrupting UTF-8 text | |
| if (strpos($decodedTitle, '=?') !== false) { | |
| $decodedTitle = mb_decode_mimeheader($decodedTitle); | |
| } | |
| $title = $entity_id . ' - ' . $decodedTitle; |
| public function testUtf8WithMalformedMimeEncodedString() | ||
| { | ||
| $wrapper = new ImapWrapper(); | ||
| // Malformed MIME-encoded string from the error report | ||
| $text = '=?iso-8859-1?Q?axz5ZAFym5luZoxqfgeds8xO/E+PtRicCu3CXJTfFFl7/aub8+5SDA59PR? =?iso-'; | ||
|
|
||
| // This should not throw an exception | ||
| $result = $wrapper->utf8($text); | ||
|
|
||
| // Should return a string (may be the same as input if imap_utf8 can't decode it) | ||
| $this->assertIsString($result); | ||
| } |
There was a problem hiding this comment.
The test verifies that the function doesn't throw an exception and returns a string, but it doesn't validate what the decoded output is for the malformed MIME string. Consider adding an assertion to check that MIME markers are removed from the result, similar to the ThreadFolderManagerTest assertions at lines 284-286, to ensure the fallback decoding logic works as expected.
IMAP operations were failing with "Invalid quoted-printable sequence" when thread titles containing malformed MIME-encoded strings (from email subjects) were used as folder names.
Changes
ThreadFolderManager: Decode MIME headers before generating folder names
=?markermb_decode_mimeheader()only when needed to preserve UTF-8 textImapWrapper: Add defensive error handling in
utf8()imap_utf8()callmb_decode_mimeheader()on decode failuresExample
Before this fix, a thread title like:
Would be passed directly to
imap_mail_move(), causing the IMAP server to fail decoding it.After the fix, it's decoded to:
With all MIME markers removed and invalid folder characters sanitized.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.