Skip to content

Fix send_all() silently truncating typed memoryviews after partial writes - #3550

Open
Nacl-HCl wants to merge 1 commit into
python-trio:mainfrom
Nacl-HCl:fix-send-all-typed-memoryview-3543
Open

Nacl-HCl wants to merge 1 commit into
python-trio:mainfrom
Nacl-HCl:fix-send-all-typed-memoryview-3543

Conversation

@Nacl-HCl

Copy link
Copy Markdown

Problem

SocketStream.send_all() and POSIX FdStream.send_all() used the byte counts returned by send() / os.write() as memoryview element offsets. For a contiguous memoryview with itemsize > 1, a partial write could therefore skip data and return successfully without sending the full buffer — a silent truncation (reproduced: 1 MiB payload, only 512 KiB sent, no error).

What changed

  • src/trio/_highlevel_socket.py (SocketStream.send_all) and src/trio/_unix_pipes.py (FdStream.send_all): after the existing empty-view early return, cast the view to a one-dimensional byte view (cast("B")) before entering the retry loop, so slice offsets and write return values are both in bytes. Empty views keep the old path because cast() rejects strided views, even empty ones.
  • Added regression tests forcing 3-byte partial writes for both paths, plus coverage that empty (and empty strided) views are still accepted.
  • Added newsfragments/3543.bugfix.rst.

Verification

  • Reproduced the truncation on unmodified main for both paths (socket: 16384 → 8194 bytes; pipe: 1 MiB → 512 KiB).
  • New tests fail on unmodified main (2 failed) and pass with the fix (2 passed).
  • test_highlevel_socket.py (8 passed), test_unix_pipes.py (12 passed), test_highlevel_generic.py (2 passed); ruff check and ruff format clean.

Fixes #3543.

Note (out of scope): _windows_pipes.py's send_all asserts written == len(data), which compares bytes written against the element count — also wrong for typed views, but fail-loud rather than silent, and I can't verify a Windows change from here. Happy to follow up if wanted.

@codecov

codecov Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00000%. Comparing base (7e0dc2d) to head (413fe01).

Additional details and impacted files
@@                 Coverage Diff                  @@
##                main        #3550         +/-   ##
====================================================
+ Coverage   98.16743%   100.00000%   +1.83257%     
====================================================
  Files            128          128                 
  Lines          19972        19638        -334     
  Branches        1399         1340         -59     
====================================================
+ Hits           19606        19638         +32     
+ Misses           359            0        -359     
+ Partials           7            0          -7     
Files with missing lines Coverage Δ
src/trio/_highlevel_socket.py 100.00000% <100.00000%> (+2.50000%) ⬆️
src/trio/_tests/test_highlevel_socket.py 100.00000% <100.00000%> (+0.62112%) ⬆️
src/trio/_tests/test_unix_pipes.py 100.00000% <100.00000%> (+1.33333%) ⬆️
src/trio/_unix_pipes.py 100.00000% <100.00000%> (ø)

... and 53 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Nacl-HCl

Copy link
Copy Markdown
Author

A note on the 3 failing checks (Ubuntu/macOS pypy-3.11): all failures are ssl.SSLError: Broken pipe raised in the SSL echo-server teardown thread (e.g. test_send_all_empty_string[tls13], test_full_duplex_basics), while the same tests pass elsewhere on the same runs (test_send_all_empty_string[tls12] passed on Ubuntu; Windows pypy-3.11 is fully green). These look like flaky teardown races in the SSL test harness, unrelated to this change — this PR only changes buffer slicing in SocketStream.send_all/FdStream.send_all, and SSLStream has its own send_all which is untouched. Happy to rebase or adjust if a maintainer sees a real connection I'm missing.

@CoolCat467 CoolCat467 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@Nacl-HCl
Nacl-HCl force-pushed the fix-send-all-typed-memoryview-3543 branch from 391c7d3 to ecae103 Compare November 9, 2026 02:43
@CoolCat467 CoolCat467 added the LLM Slop Entirely generated by a large language model with little or no human oversight label Nov 10, 2026
@Nacl-HCl
Nacl-HCl force-pushed the fix-send-all-typed-memoryview-3543 branch from ecae103 to 80eaa07 Compare November 11, 2026 08:18
…ites

SocketStream.send_all() and POSIX FdStream.send_all() used the byte counts
returned by send()/os.write() as memoryview element offsets. For a
contiguous memoryview with itemsize > 1, a partial write could skip data
and return successfully without sending the full buffer.

Cast non-empty views to a one-dimensional byte view before slicing, so
offsets and write return values are both in bytes. Empty views keep the
previous early-return path because cast() rejects strided views, even
empty ones.

Fixes python-trio#3543
@Nacl-HCl
Nacl-HCl force-pushed the fix-send-all-typed-memoryview-3543 branch from 80eaa07 to 413fe01 Compare November 15, 2026 08:04

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LLM Slop Entirely generated by a large language model with little or no human oversight

Projects

None yet

Development

Successfully merging this pull request may close these issues.

send_all can silently truncate typed memoryviews after partial writes

2 participants