From 407ef8db530276a0a627650df2f76894930d2690 Mon Sep 17 00:00:00 2001 From: Fernando Celmer Date: Sun, 17 May 2026 18:13:53 -0300 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=AA=B2=20BUG-#67:=20Use=20UID=20EXPUN?= =?UTF-8?q?GE=20in=20MailBox.move=20fallback=20and=20narrow=20except?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- email_profile/clients/imap/mailbox.py | 18 ++++++++++++++---- tests/clients/imap/test_write_ops.py | 7 +++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/email_profile/clients/imap/mailbox.py b/email_profile/clients/imap/mailbox.py index 49fee95..15501a6 100644 --- a/email_profile/clients/imap/mailbox.py +++ b/email_profile/clients/imap/mailbox.py @@ -184,17 +184,27 @@ def copy(self, target: UIDLike, destination: str) -> None: def move(self, target: UIDLike, destination: str) -> None: """Move a message into another mailbox. - Prefers ``UID MOVE`` (RFC 6851). Falls back to copy + delete + expunge - when the server does not advertise the MOVE extension. + Prefers ``UID MOVE`` (RFC 6851). Falls back to copy + delete + + ``UID EXPUNGE`` (RFC 4315) when the server does not advertise MOVE. + Only ``imaplib.IMAP4.error`` is treated as the missing-MOVE signal — + network/auth/quota errors propagate. """ + from email_profile.core.errors import IMAPError + Status.state(self._client.select(_quote(self.name))) uid = _uid_of(target) try: Status.state(self._client.uid("MOVE", uid, destination)) - except Exception: + except imaplib.IMAP4.error: Status.state(self._client.uid("COPY", uid, destination)) Status.state(self._client.uid("STORE", uid, "+FLAGS", "\\Deleted")) - Status.state(self._client.expunge()) + try: + Status.state(self._client.uid("EXPUNGE", uid)) + except imaplib.IMAP4.error as exc: + raise IMAPError( + "Server lacks MOVE and UIDPLUS; message was copied and " + "flagged \\Deleted but not expunged." + ) from exc def create(self) -> None: """Create this mailbox on the server.""" diff --git a/tests/clients/imap/test_write_ops.py b/tests/clients/imap/test_write_ops.py index 4cd67a8..096bc11 100644 --- a/tests/clients/imap/test_write_ops.py +++ b/tests/clients/imap/test_write_ops.py @@ -79,12 +79,14 @@ def test_move_prefers_uid_move(self): self.assertEqual(call.args, ("MOVE", "42", "Archive")) def test_move_falls_back_to_copy_delete(self): + import imaplib + calls = [] def fake_uid(command, *args): calls.append((command, args)) if command.upper() == "MOVE": - raise Exception("MOVE not supported") + raise imaplib.IMAP4.error("MOVE not supported") return ("OK", [b"Done"]) self.fake.uid.side_effect = fake_uid @@ -94,7 +96,8 @@ def fake_uid(command, *args): self.assertIn("MOVE", commands) self.assertIn("COPY", commands) self.assertIn("STORE", commands) - self.fake.expunge.assert_called_once() + self.assertIn("EXPUNGE", commands) + self.fake.expunge.assert_not_called() class TestMailboxAdmin(_WriteTest): From b1226512e1c131d245f2d4df033df7782c6def94 Mon Sep 17 00:00:00 2001 From: Fernando Celmer Date: Sun, 17 May 2026 18:13:58 -0300 Subject: [PATCH 2/3] =?UTF-8?q?=E2=9D=A4=EF=B8=8F=20TEST-#67:=20Add=20Mail?= =?UTF-8?q?Box.move=20fallback,=20UIDPLUS=20and=20error-propagation=20test?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/clients/imap/test_mailbox.py | 55 ++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/tests/clients/imap/test_mailbox.py b/tests/clients/imap/test_mailbox.py index 3f873bb..8940e85 100644 --- a/tests/clients/imap/test_mailbox.py +++ b/tests/clients/imap/test_mailbox.py @@ -1,7 +1,9 @@ +import imaplib from unittest import TestCase from unittest.mock import patch from email_profile import Email, Message, Q, Query +from email_profile.core.errors import IMAPError from tests.conftest import SAMPLE_RFC822, make_fake_client @@ -81,3 +83,56 @@ def test_returns_appended_uid_when_supported(self): def test_returns_none_without_uidplus(self): self.fake.append.return_value = ("OK", [b"APPEND completed"]) self.assertIsNone(self.app.inbox.append(SAMPLE_RFC822)) + + +class TestMailBoxMove(TestCase): + def setUp(self): + self.fake = make_fake_client() + self._patcher = patch( + "email_profile.clients.imap.client.imaplib.IMAP4_SSL", + return_value=self.fake, + ) + self._patcher.start() + self.app = Email("imap.x", "u", "p").connect() + + def tearDown(self): + self.app.close() + self._patcher.stop() + + def test_uses_move_when_supported(self): + self.app.inbox.move("42", "Archive") + calls = [c.args[0] for c in self.fake.uid.call_args_list] + self.assertIn("MOVE", calls) + self.assertNotIn("COPY", calls) + self.fake.expunge.assert_not_called() + + def test_falls_back_to_copy_uid_expunge(self): + def side(command, *args): + cmd = command.upper() + if cmd == "MOVE": + raise imaplib.IMAP4.error("MOVE not supported") + return ("OK", [b"Done"]) + + self.fake.uid.side_effect = side + self.app.inbox.move("42", "Archive") + calls = [c.args[0] for c in self.fake.uid.call_args_list] + self.assertIn("COPY", calls) + self.assertIn("STORE", calls) + self.assertIn("EXPUNGE", calls) + self.fake.expunge.assert_not_called() + + def test_propagates_non_imap_errors(self): + self.fake.uid.side_effect = ConnectionResetError("network drop") + with self.assertRaises(ConnectionResetError): + self.app.inbox.move("42", "Archive") + + def test_raises_when_uidplus_missing(self): + def side(command, *args): + cmd = command.upper() + if cmd in {"MOVE", "EXPUNGE"}: + raise imaplib.IMAP4.error("not supported") + return ("OK", [b"Done"]) + + self.fake.uid.side_effect = side + with self.assertRaises(IMAPError): + self.app.inbox.move("42", "Archive") From 48a196c7b328b29df93a8d73f5c47ba3d70d9a58 Mon Sep 17 00:00:00 2001 From: Fernando Celmer Date: Sun, 17 May 2026 18:29:37 -0300 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=AA=B2=20BUG-#67:=20Also=20catch=20IM?= =?UTF-8?q?APError=20in=20MOVE=20fallback=20(NO/BAD=20response)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Status.state raises core.errors.IMAPError when the server returns a tagged NO/BAD response — the most common way to signal an unsupported command. The previous narrowed except only caught imaplib.IMAP4.error so MOVE fallback never ran on servers without RFC 6851. Catch (imaplib.IMAP4.error, IMAPError) in both try blocks. Added two regression tests that simulate the NO response shape Dovecot and others emit in production. --- email_profile/clients/imap/mailbox.py | 9 ++++--- tests/clients/imap/test_mailbox.py | 36 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/email_profile/clients/imap/mailbox.py b/email_profile/clients/imap/mailbox.py index 15501a6..4dbe0be 100644 --- a/email_profile/clients/imap/mailbox.py +++ b/email_profile/clients/imap/mailbox.py @@ -186,8 +186,9 @@ def move(self, target: UIDLike, destination: str) -> None: Prefers ``UID MOVE`` (RFC 6851). Falls back to copy + delete + ``UID EXPUNGE`` (RFC 4315) when the server does not advertise MOVE. - Only ``imaplib.IMAP4.error`` is treated as the missing-MOVE signal — - network/auth/quota errors propagate. + ``imaplib.IMAP4.error`` and ``IMAPError`` (raised by ``Status.state`` + on tagged ``NO``/``BAD`` responses) are treated as the missing-MOVE + signal — network/auth/quota errors propagate. """ from email_profile.core.errors import IMAPError @@ -195,12 +196,12 @@ def move(self, target: UIDLike, destination: str) -> None: uid = _uid_of(target) try: Status.state(self._client.uid("MOVE", uid, destination)) - except imaplib.IMAP4.error: + except (imaplib.IMAP4.error, IMAPError): Status.state(self._client.uid("COPY", uid, destination)) Status.state(self._client.uid("STORE", uid, "+FLAGS", "\\Deleted")) try: Status.state(self._client.uid("EXPUNGE", uid)) - except imaplib.IMAP4.error as exc: + except (imaplib.IMAP4.error, IMAPError) as exc: raise IMAPError( "Server lacks MOVE and UIDPLUS; message was copied and " "flagged \\Deleted but not expunged." diff --git a/tests/clients/imap/test_mailbox.py b/tests/clients/imap/test_mailbox.py index 8940e85..1312ce4 100644 --- a/tests/clients/imap/test_mailbox.py +++ b/tests/clients/imap/test_mailbox.py @@ -136,3 +136,39 @@ def side(command, *args): self.fake.uid.side_effect = side with self.assertRaises(IMAPError): self.app.inbox.move("42", "Archive") + + def test_falls_back_when_server_responds_no_to_move(self): + """Server signals missing MOVE via tagged ``NO`` — not an exception. + + ``Status.state`` translates ``("NO", ...)`` into ``IMAPError``; + the fallback must catch it so the COPY+STORE+UID EXPUNGE path runs. + """ + + def side(command, *args): + cmd = command.upper() + if cmd == "MOVE": + return ("NO", [b"MOVE not supported"]) + return ("OK", [b"Done"]) + + self.fake.uid.side_effect = side + self.app.inbox.move("42", "Archive") + calls = [c.args[0] for c in self.fake.uid.call_args_list] + self.assertIn("COPY", calls) + self.assertIn("STORE", calls) + self.assertIn("EXPUNGE", calls) + self.fake.expunge.assert_not_called() + + def test_raises_when_server_responds_no_to_uid_expunge(self): + """Server lacks UIDPLUS and replies ``NO`` to UID EXPUNGE.""" + + def side(command, *args): + cmd = command.upper() + if cmd == "MOVE": + return ("NO", [b"MOVE not supported"]) + if cmd == "EXPUNGE": + return ("NO", [b"UIDPLUS not supported"]) + return ("OK", [b"Done"]) + + self.fake.uid.side_effect = side + with self.assertRaises(IMAPError): + self.app.inbox.move("42", "Archive")