python / python/cpython

Windows single-file mailbox rewrite can remove the mailbox when the fallback rename fails

Open
#156,699 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

OS-windows stdlib topic-email type-bug
Dominant language
Python
Stars
77.2k
Forks
35.9k
PR merge metrics
PR metrics pending

Description

Bug report

Bug description

Summary

On Windows, _singlefileMailbox.flush() cannot rename a temporary rewrite over an existing mailbox with os.rename(). Its FileExistsError fallback deletes the original mailbox and then performs a second rename. If that second filesystem operation fails, the mailbox pathname has already been removed.

The affected mailbox classes are mailbox.mbox, mailbox.MMDF, and mailbox.Babyl. The temporary replacement normally remains, so the contents are not necessarily irretrievable, but the original mailbox pathname is missing. This is a Windows-specific failure path because it depends on Windows os.rename() refusing to replace an existing destination.

Reproduction Code
import errno
import glob
import mailbox
import os
import tempfile
from unittest.mock import patch

with tempfile.TemporaryDirectory() as directory:
    path = os.path.join(directory, "mailbox")
    box = mailbox.mbox(path)
    first = box.add(b"Subject: first\n\nfirst\n")
    box.add(b"Subject: second\n\nsecond\n")
    box.flush()
    box.remove(first)

    # Simulate Windows rejecting the first replacement attempt, followed by
    # an I/O failure while the fallback performs its second rename.
    with patch(
        "mailbox.os.rename",
        side_effect=[
            FileExistsError(errno.EEXIST, "target exists"),
            OSError(errno.EIO, "injected second rename failure"),
        ],
    ):
        try:
            box.flush()
        except OSError as error:
            print("flush:", type(error).__name__, error.errno)

    print("mailbox exists:", os.path.exists(path))
    print(
        "temporary files:",
        [os.path.basename(name) for name in glob.glob(path + ".*")],
    )

Observed on a Windows CPython build:

flush: OSError 5
mailbox exists: False
temporary files: ['mailbox.1788174401.yuu.26684']
Actual Behavior

After the first os.rename() raises FileExistsError, flush() removes the existing mailbox pathname. If the second os.rename() then fails, the exception propagates after the original mailbox has already been removed.

The rewritten temporary file normally remains, but the mailbox is no longer available at its original pathname.

Expected Behavior

If replacement of the rewritten mailbox fails, the original mailbox pathname should remain intact. Replacing an existing mailbox should not require explicitly removing the destination before installing the rewritten file.

CPython versions tested on

CPython main branch

Operating systems tested on

Windows

Linked PRs
  • gh-156700

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in the mailbox module at _singlefileMailbox.flush() and review the replacement path for mailbox.mbox, mailbox.MMDF, and mailbox.Babyl. Reproduce the two mocked os.rename failures from the issue, then add or update a regression test so the original mailbox pathname remains when replacement fails; review linked PR gh-156700 before starting.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.