Skip to content

Preserve concurrent attachment additions and validate stored text - #548

Merged
yhay81 merged 1 commit into
mainfrom
claude/539-attachment-races
Nov 1, 2026
Merged

yhay81 merged 1 commit into
mainfrom
claude/539-attachment-races

Conversation

@yhay81

@yhay81 yhay81 commented Nov 1, 2026

Copy link
Copy Markdown
Owner

Closes #539

Changes

  • Attachments.add and Attachments.__delitem__ read and plan the /EmbeddedFiles name tree inside their transaction, so add/add, add/delete, and delete/add races no longer write a stale snapshot. Existence and the 4,096-entry bound are checked under the lock as well; a raced duplicate name raises PdfError, and a raced double delete raises KeyError.
  • Attachment.write resolves its file specification and stream under the transaction lock; after a concurrent delete it raises StaleObjectError instead of editing a stream that another attachment shares, and after a concurrent replacement it writes the new attachment.
  • The name, file name, and description are converted to exact str with str.__str__ after validation, so the stored text is the validated text and str subclass overrides (__len__, encode, __str__, __eq__) are never called by the writer. The empty-name check uses str.__len__.
  • _validate_data reads data through memoryview, which a bytes subclass cannot override, checks nbytes against max_size, and writes tobytes(). Data that is not bytes-like (for example an int or a list) now raises TypeError. Before this change, bytes(3) was accepted silently as three zero bytes.

Acceptance

  • Deterministic concurrency tests cover overlapping additions and deletion and write races; no unrelated entry or shared stream is lost or changed. In each test, the main thread holds a transaction while a worker reads the old version and then blocks. The tests cover add/add, add vs. delete, delete vs. add, delete/delete, a duplicate name, write vs. delete on a shared stream, and write vs. replace.
  • Filename and description bounds cannot be bypassed by str subclasses or large Unicode input, and serialization uses the validated values (test_str_subclasses_write_their_validated_text, test_large_unicode_text_is_refused_in_utf8_without_changes).
  • Attachments.add and Attachment.write cannot exceed max_size through a bytes subclass, and write the validated bytes (test_bytes_subclasses_cannot_exceed_max_size, test_bytes_subclasses_write_their_buffer).
  • Failures leave the document and history unchanged. The tests check the attachment names, contents, and doc.history.steps. Existing attachment tests and python3 tools/check.py pass. Current-head CI is pending.

All 14 new tests fail on main and pass with this change, so each race and bypass was reproduced first.

🤖 Generated with Claude Code

…ed input

Attachments.add and deletion now read the EmbeddedFiles name tree inside
their transaction, so a concurrent addition or deletion is no longer
overwritten by a stale snapshot. Attachment.write resolves its stream under
the lock, so a concurrent delete cannot leave it writing a stream that
another attachment shares. Names, file names, and descriptions are written
as the exact str just validated, and data is read through the buffer
protocol, so str and bytes subclass overrides cannot bypass the input-size
and max_size bounds.

Closes #539

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yhay81
yhay81 merged commit 5115da9 into main Nov 1, 2026
5 checks passed
@yhay81
yhay81 deleted the claude/539-attachment-races branch November 1, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preserve concurrent attachment additions and validate stored text

1 participant