From 0855c8ace4d95d8c5170b4f1d4d7fd3354687e95 Mon Sep 17 00:00:00 2001 From: John Pastore <50429657+cbbm142@users.noreply.github.com> Date: Tue, 19 May 2026 11:46:29 -0400 Subject: [PATCH 1/5] Preserve write buffer when raw.write() returns None on blocked socket Add a check in cheroot.makefile.BufferedWriter._flush_unlocked to prevent clearing of the write buffer when raw.write() returns None due to a blocked stream. Also limits each write call to SOCK_WRITE_BLOCKSIZE bytes, which was defined but not previously used in this method. --- cheroot/makefile.py | 8 ++++++-- cheroot/test/test_makefile.py | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/cheroot/makefile.py b/cheroot/makefile.py index f5780a1ede..0376655548 100644 --- a/cheroot/makefile.py +++ b/cheroot/makefile.py @@ -26,13 +26,17 @@ def write(self, b): def _flush_unlocked(self): self._checkClosed('flush of closed file') while self._write_buf: + n = None try: # ssl sockets only except 'bytes', not bytearrays # so perhaps we should conditionally wrap this for perf? - n = self.raw.write(bytes(self._write_buf)) + n = self.raw.write( + bytes(self._write_buf[:SOCK_WRITE_BLOCKSIZE]), + ) except io.BlockingIOError as e: n = e.characters_written - del self._write_buf[:n] + if n: + del self._write_buf[:n] class StreamReader(io.BufferedReader): diff --git a/cheroot/test/test_makefile.py b/cheroot/test/test_makefile.py index d65d4ea268..7aea33860a 100644 --- a/cheroot/test/test_makefile.py +++ b/cheroot/test/test_makefile.py @@ -51,3 +51,39 @@ def test_bytes_written(): wfile = makefile.MakeFile(sock, 'w') wfile.write(b'bar') assert wfile.bytes_written == 3 + + +def test_flush_preserves_data_when_raw_write_returns_none(): + """_flush_unlocked() must not treat None from raw.write() as a byte count. + + io.RawIOBase.write() returns None when a non-blocking socket cannot accept + data. del self._write_buf[:None] is equivalent to del self._write_buf[:] + which silently clears the entire buffer, truncating the response without + raising an exception. + """ + data = b'x' * 32768 # larger than SOCK_WRITE_BLOCKSIZE to stress the loop + + sock = MockSocket() + wfile = makefile.MakeFile(sock, 'w') + wfile._write_buf.extend(data) + + written = bytearray() + call_count = 0 + + def mock_raw_write(b): + nonlocal call_count + call_count += 1 + if call_count == 1: + return ( + None # simulates socket returning None on first blocked write + ) + written.extend(b) + return len(b) + + wfile.raw.write = mock_raw_write + wfile._flush_unlocked() + + assert bytes(written) == data, ( + f'Expected {len(data)} bytes but only {len(written)} reached raw.write(): ' + 'buffer was silently discarded when raw.write() returned None' + ) From 844bc5173cd1f048158feae851f3d016d0e26db3 Mon Sep 17 00:00:00 2001 From: John Pastore <50429657+cbbm142@users.noreply.github.com> Date: Tue, 19 May 2026 11:46:29 -0400 Subject: [PATCH 2/5] Fix linting issues in makefile test Updated test to allow Flake8/pre-commit checks to pass successfully. --- cheroot/test/test_makefile.py | 43 ++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/cheroot/test/test_makefile.py b/cheroot/test/test_makefile.py index 7aea33860a..6a65dd574e 100644 --- a/cheroot/test/test_makefile.py +++ b/cheroot/test/test_makefile.py @@ -53,7 +53,26 @@ def test_bytes_written(): assert wfile.bytes_written == 3 -def test_flush_preserves_data_when_raw_write_returns_none(): +class _RawWriteBlockOnce: + """Mock raw.write() that returns None on the first call, then writes normally.""" + + def __init__(self): + """Initialize _RawWriteBlockOnce.""" + self.call_count = 0 + self.written = bytearray() + + def __call__(self, chunk): + """Return None on first call to simulate a blocked socket write.""" + self.call_count += 1 + if self.call_count == 1: + return ( + None # simulates socket returning None on first blocked write + ) + self.written.extend(chunk) + return len(chunk) + + +def test_flush_when_raw_write_returns_none(): """_flush_unlocked() must not treat None from raw.write() as a byte count. io.RawIOBase.write() returns None when a non-blocking socket cannot accept @@ -61,29 +80,17 @@ def test_flush_preserves_data_when_raw_write_returns_none(): which silently clears the entire buffer, truncating the response without raising an exception. """ - data = b'x' * 32768 # larger than SOCK_WRITE_BLOCKSIZE to stress the loop + data = b'x' * (makefile.SOCK_WRITE_BLOCKSIZE * 2) # stress the write loop sock = MockSocket() wfile = makefile.MakeFile(sock, 'w') wfile._write_buf.extend(data) - written = bytearray() - call_count = 0 - - def mock_raw_write(b): - nonlocal call_count - call_count += 1 - if call_count == 1: - return ( - None # simulates socket returning None on first blocked write - ) - written.extend(b) - return len(b) - - wfile.raw.write = mock_raw_write + mock = _RawWriteBlockOnce() + wfile.raw.write = mock wfile._flush_unlocked() - assert bytes(written) == data, ( - f'Expected {len(data)} bytes but only {len(written)} reached raw.write(): ' + assert bytes(mock.written) == data, ( + f'Expected {len(data)} bytes but only {len(mock.written)} reached raw.write(): ' 'buffer was silently discarded when raw.write() returned None' ) From 5eaf1acbdf4f9ad1b8ec24df47882dfff25d809d Mon Sep 17 00:00:00 2001 From: John Pastore <50429657+cbbm142@users.noreply.github.com> Date: Tue, 19 May 2026 14:20:15 -0400 Subject: [PATCH 3/5] Fix spell check failures Updated allowlist to include buf, io, and RawIOBase --- docs/spelling_wordlist.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 06b204518e..981a6b3f97 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -5,6 +5,7 @@ backports bugfixes builtin b'xb +buf compat config conftest @@ -22,6 +23,7 @@ hardcoded hostname inclusivity intersphinx +io iterable linter linters @@ -48,6 +50,7 @@ preconfigure py pytest pythonic +RawIOBase readonly rebase Refactor From d3e8258f38135ebfa20ada365343676fc381ad62 Mon Sep 17 00:00:00 2001 From: John Pastore <50429657+cbbm142@users.noreply.github.com> Date: Wed, 20 May 2026 07:08:27 -0400 Subject: [PATCH 4/5] Add file for towncrier Added changelog entry for change #822 --- docs/changelog-fragments.d/822.bugfix.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 docs/changelog-fragments.d/822.bugfix.rst diff --git a/docs/changelog-fragments.d/822.bugfix.rst b/docs/changelog-fragments.d/822.bugfix.rst new file mode 100644 index 0000000000..97493eeb59 --- /dev/null +++ b/docs/changelog-fragments.d/822.bugfix.rst @@ -0,0 +1,3 @@ +Fixed a bug that could cause premature clearing of the write buffer when a socket write is blocked. + +-- by :user:`cbbm142` From d84185bce70acc4b234a6733ef8a0b432b8f7c82 Mon Sep 17 00:00:00 2001 From: John Pastore <50429657+cbbm142@users.noreply.github.com> Date: Wed, 17 Jun 2026 10:04:20 -0400 Subject: [PATCH 5/5] Use select() to retry blocked socket writes When raw.write() returns None on a blocked non-blocking socket, use select() to wait up to SOCK_WRITE_TIMEOUT seconds for the socket to become writable before retrying. This avoids both the original silent buffer truncation bug and the infinite spin loop that the previous fix could produce. If the socket stays blocked past the timeout, raise BlockingIOError with the write buffer preserved. Co-Authored-By: Claude Opus 4.6 (1M context) --- cheroot/makefile.py | 20 ++++++++- cheroot/makefile.pyi | 1 + cheroot/test/test_makefile.py | 82 +++++++++++++++++++++++++++++------ 3 files changed, 87 insertions(+), 16 deletions(-) diff --git a/cheroot/makefile.py b/cheroot/makefile.py index 0376655548..59c00e9769 100644 --- a/cheroot/makefile.py +++ b/cheroot/makefile.py @@ -2,12 +2,16 @@ # prefer slower Python-based io module import _pyio as io +import select import socket # Write only 16K at a time to sockets SOCK_WRITE_BLOCKSIZE = 16384 +# Seconds to wait for a blocked socket to become writable +SOCK_WRITE_TIMEOUT = 10 + class BufferedWriter(io.BufferedWriter): """Faux file object attached to a socket object.""" @@ -35,8 +39,20 @@ def _flush_unlocked(self): ) except io.BlockingIOError as e: n = e.characters_written - if n: - del self._write_buf[:n] + if n is None: + _, writable, _ = select.select( + [], + [self.raw], + [], + SOCK_WRITE_TIMEOUT, + ) + if not writable: + raise io.BlockingIOError( + 0, + 'raw stream blocked; no bytes written', + ) + continue + del self._write_buf[:n] class StreamReader(io.BufferedReader): diff --git a/cheroot/makefile.pyi b/cheroot/makefile.pyi index 3f5ea2756b..8e8d57d73f 100644 --- a/cheroot/makefile.pyi +++ b/cheroot/makefile.pyi @@ -1,6 +1,7 @@ import io SOCK_WRITE_BLOCKSIZE: int +SOCK_WRITE_TIMEOUT: int class BufferedWriter(io.BufferedWriter): def write(self, b): ... diff --git a/cheroot/test/test_makefile.py b/cheroot/test/test_makefile.py index 6a65dd574e..f0582dba15 100644 --- a/cheroot/test/test_makefile.py +++ b/cheroot/test/test_makefile.py @@ -54,7 +54,7 @@ def test_bytes_written(): class _RawWriteBlockOnce: - """Mock raw.write() that returns None on the first call, then writes normally.""" + """Mock raw.write() returning None once, then writing normally.""" def __init__(self): """Initialize _RawWriteBlockOnce.""" @@ -62,25 +62,41 @@ def __init__(self): self.written = bytearray() def __call__(self, chunk): - """Return None on first call to simulate a blocked socket write.""" + """Return None on first call to simulate a blocked write.""" self.call_count += 1 if self.call_count == 1: - return ( - None # simulates socket returning None on first blocked write - ) + return None self.written.extend(chunk) return len(chunk) + def fileno(self): + """Return a fake fd for select().""" + return -1 -def test_flush_when_raw_write_returns_none(): - """_flush_unlocked() must not treat None from raw.write() as a byte count. - io.RawIOBase.write() returns None when a non-blocking socket cannot accept - data. del self._write_buf[:None] is equivalent to del self._write_buf[:] - which silently clears the entire buffer, truncating the response without - raising an exception. +class _RawWriteBlockAlways: + """Mock raw.write() that always returns None.""" + + def __init__(self): + """Initialize _RawWriteBlockAlways.""" + self.call_count = 0 + + def __call__(self, chunk): + """Return None to simulate a permanently blocked socket.""" + self.call_count += 1 + + def fileno(self): + """Return a fake fd for select().""" + return -1 + + +def test_flush_recovers_from_temporary_block(monkeypatch): + """_flush_unlocked() retries after select when raw.write() returns None. + + A temporarily blocked socket should recover once select() reports + the socket is writable again, delivering all buffered data. """ - data = b'x' * (makefile.SOCK_WRITE_BLOCKSIZE * 2) # stress the write loop + data = b'x' * (makefile.SOCK_WRITE_BLOCKSIZE * 2) sock = MockSocket() wfile = makefile.MakeFile(sock, 'w') @@ -88,9 +104,47 @@ def test_flush_when_raw_write_returns_none(): mock = _RawWriteBlockOnce() wfile.raw.write = mock + + # select() reports writable immediately + monkeypatch.setattr( + 'cheroot.makefile.select.select', + lambda _rlist, wlist, _xlist, _timeout: ([], wlist, []), + ) wfile._flush_unlocked() assert bytes(mock.written) == data, ( - f'Expected {len(data)} bytes but only {len(mock.written)} reached raw.write(): ' - 'buffer was silently discarded when raw.write() returned None' + 'all buffered data should be written after select retry' + ) + + +def test_flush_raises_on_sustained_block(monkeypatch): + """_flush_unlocked() raises BlockingIOError after select timeout. + + If the socket stays blocked past SOCK_WRITE_TIMEOUT, the write + buffer must be preserved and BlockingIOError raised. + """ + import io + + import pytest + + data = b'x' * makefile.SOCK_WRITE_BLOCKSIZE + + sock = MockSocket() + wfile = makefile.MakeFile(sock, 'w') + wfile._write_buf.extend(data) + + mock = _RawWriteBlockAlways() + wfile.raw.write = mock + + # select() reports not writable (timeout) + monkeypatch.setattr( + 'cheroot.makefile.select.select', + lambda _rlist, _wlist, _xlist, _timeout: ([], [], []), + ) + + with pytest.raises(io.BlockingIOError): + wfile._flush_unlocked() + + assert len(wfile._write_buf) == len(data), ( + 'write buffer must be preserved when socket stays blocked' )