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.

…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
@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 (4630d02) to head (391c7d3).

Additional details and impacted files
@@               Coverage Diff               @@
##                 main        #3550   +/-   ##
===============================================
  Coverage   100.00000%   100.00000%           
===============================================
  Files             128          128           
  Lines           19474        19524   +50     
  Branches         1323         1328    +5     
===============================================
+ Hits            19474        19524   +50     
Files with missing lines Coverage Δ
src/trio/_highlevel_socket.py 100.00000% <100.00000%> (ø)
src/trio/_tests/test_highlevel_socket.py 100.00000% <100.00000%> (ø)
src/trio/_tests/test_unix_pipes.py 100.00000% <100.00000%> (ø)
src/trio/_unix_pipes.py 100.00000% <100.00000%> (ø)
🚀 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.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

send_all can silently truncate typed memoryviews after partial writes

1 participant