[PR #1217] [CLOSED] When reading literal_data from mail body from IMAP, use byte count instead of string-length count. #1348

New issue

Closed

opened 2026-02-25 21:38:00 +03:00 by kerem · 0 comments

kerem commented

2026-02-25 21:38:00 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cypht-org/cypht/pull/1217
Author: @indridieinarsson
Created: 9/1/2024
Status: ❌ Closed

Base: master ← Head: issue_1119

📝 Commits (10+)

e696471 Use single-byte string-length to measure length of literal data
6768bcc remove commented-out code
9862b5a Remove comment.
fd9a046 Merge branch 'cypht-org:master' into issue_1119
bd8b85c Wait_with_folder_list when reloading folders. We're reloading the page, might have to wait.
342ddcb Undo changes to reload_folder_test.
2de5e36 Implicitly wait 10 seconds after reloading all messages
f21011c Undo changes to selenium test. Fix wasn't working, no idea why.
5152540 Try to kick the main_menu reference to update.
e464633 stupid bug

📊 Changes

2 files changed (+23 additions, -8 deletions)

View changed files

📝 modules/imap/hm-imap-base.php (+7 -6)
📝 tests/selenium/folder_list.py (+16 -2)

📄 Description

Pullrequest

This is a first attempt to fix issue #1119.
I'm not very proficient in php, and not knowledgeable about imap, so for the love of all that is good and holy, review and test this properly.

Take a look at this chunk from hm-imap-base.php:

elseif ($line[$i] == '{') {
$end = mb_strpos($line, '}');
if ($end !== false) {
$literal_size = mb_substr($line, ($i + 1), ($end - $i - 1));
}
$lit_result = $this->read_literal($literal_size, $max, $current_size, $line_length);
$chunk = $lit_result[0];

If I understand this writing correctly: https://www.rfc-editor.org/rfc/rfc3501#section-4.3, the number in curly braces that is being read into $literal_size, is the number of bytes in the email body, which is then sent over the wire right away.
Therefore, all string-size calculations in read_literal() should use the strsize rather than mb_strsize command. My theory is that the issue #1119 is caused by this: the number of bytes read are underestimated since mb_strsize is smaller than strsize for real multibyte strings. We then end up trying to read strings from the server after the server has sent the entire message, causing the code to hang.

Issues

Might possibly fix #1119

Checklist

This touches the code that reads the email body, pretty central for an email client. This PR will change behaviour in cases when the body of the message contains multibyte characters (ÞþæÆöÖðÐáÁóÓ etc.).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cypht-org/cypht/pull/1217 **Author:** [@indridieinarsson](https://github.com/indridieinarsson) **Created:** 9/1/2024 **Status:** ❌ Closed **Base:** `master` ← **Head:** `issue_1119` --- ### 📝 Commits (10+) - [`e696471`](https://github.com/cypht-org/cypht/commit/e6964712663f4a8149cf8d7cc947e1e7a2927603) Use single-byte string-length to measure length of literal data - [`6768bcc`](https://github.com/cypht-org/cypht/commit/6768bcc276bf89f901bf6fa3ea1ac0b0f73ac3e2) remove commented-out code - [`9862b5a`](https://github.com/cypht-org/cypht/commit/9862b5a1321b1c79488a58b07f7c30def2762a19) Remove comment. - [`fd9a046`](https://github.com/cypht-org/cypht/commit/fd9a04693123e43cb16e10222ed8a916ac267a4d) Merge branch 'cypht-org:master' into issue_1119 - [`bd8b85c`](https://github.com/cypht-org/cypht/commit/bd8b85c716f9d38a51860ab8a5ed9c3ce725ab35) Wait_with_folder_list when reloading folders. We're reloading the page, might have to wait. - [`342ddcb`](https://github.com/cypht-org/cypht/commit/342ddcb6f8ea6bf91969d2391c9b4f7bbca1b3ab) Undo changes to reload_folder_test. - [`2de5e36`](https://github.com/cypht-org/cypht/commit/2de5e3660faa8545edbe9e00fca1469dd8314b97) Implicitly wait 10 seconds after reloading all messages - [`f21011c`](https://github.com/cypht-org/cypht/commit/f21011cc90e047d4d490906a0a3be3660c29b365) Undo changes to selenium test. Fix wasn't working, no idea why. - [`5152540`](https://github.com/cypht-org/cypht/commit/515254011022773b4bfb1b957056b04f6725077e) Try to kick the main_menu reference to update. - [`e464633`](https://github.com/cypht-org/cypht/commit/e464633eb603354915fb6ab6d5f6bbb0474c75af) stupid bug ### 📊 Changes **2 files changed** (+23 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `modules/imap/hm-imap-base.php` (+7 -6) 📝 `tests/selenium/folder_list.py` (+16 -2) </details> ### 📄 Description ## Pullrequest This is a first attempt to fix issue #1119. I'm not very proficient in php, and not knowledgeable about imap, so for the love of all that is good and holy, review and test this properly. Take a look at this chunk from hm-imap-base.php: > elseif ($line[$i] == '{') { > $end = mb_strpos($line, '}'); > if ($end !== false) { > $literal_size = mb_substr($line, ($i + 1), ($end - $i - 1)); > } > $lit_result = $this->read_literal($literal_size, $max, $current_size, $line_length); > $chunk = $lit_result[0]; > If I understand this writing correctly: https://www.rfc-editor.org/rfc/rfc3501#section-4.3, the number in curly braces that is being read into $literal_size, is the number of bytes in the email body, which is then sent over the wire right away. Therefore, all string-size calculations in `read_literal()` should use the `strsize` rather than `mb_strsize` command. My theory is that the issue #1119 is caused by this: the number of bytes read are underestimated since `mb_strsize` is smaller than `strsize` for real multibyte strings. We then end up trying to read strings from the server after the server has sent the entire message, causing the code to hang. ### Issues - Might possibly fix #1119 ### Checklist This touches the code that reads the email body, pretty central for an email client. This PR will change behaviour in cases when the body of the message contains multibyte characters (ÞþæÆöÖðÐáÁóÓ etc.). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>