GeezerVison

a comment that this is a geometric increase in read size (never more than doubling our current the length of data per loop iteration) could be good. otherwise it takes some staring at the code to understand what it is doing with the delta.

not a big deal though given CGIHTTPRequestHandler is gone in 3.15 onwards. :)

the timeout=0 fourth arg here makes this non-blocking. the loop won't wait for further data to arrive. so we wind up not processing all of the data.

(Claude Code Opus 4.5 looking at the buildbot failure, issue, pr & branch triaged this FWIW)

why select at all here given we just want to block on reading another chunk? if the socket closes or errors, that'd return anyways.

Quick PR to make it blocking: #142176

What should this do if the data doesn't arrive?
Wait forever, as in did before the fix?
Or doses this PR also meant fix the DoS? (I don't see how, is there a timeout setting or an implicit one?)

It was so long ago that I don't remember my train of thought, but playing around with the code I think I've recreated it.

I tested this code on Linux with have_fork = False. The tests failed because the underlying file object is unbuffered and read(n) can return less than n bytes. So the loop differs from loops in other fixes -- it continues until it reads 0 bytes. select() is needed to avoid blocking. Until now all worked on Linux and Windows on my computer and in CI tests. Perhaps this depends on the system load or on non-debug build (I only tested with the debug build).

Perhaps we need to use non-zero timeout (self.timeout?) and set it to non-None for tests.

xmlrpc/server.py has a similar code path

but without a select. The last change there was ec1712a in 2012 adding the break if no data was read. So without a timeout it will suffer the same problem if content-length lies and less data is sent?

OTOH, does anybody out there operate a server without a timeout?

Wait forever, as in did before the fix?

yes. this is intentionally a blocking read until sufficient data arrives or the connection dies. :)

fwiw, i missed this myself in the first code review. cursed unnamed parameters and presumptions about what 0 means. 😁

we do not need to add timeout logic here. just remove the select. this path was always intended to block until it got enough data or the socket is closed. the issue being protected against has nothing to do with blocking by holding a connection open, it's just about memory allocation.

This 1990s era server code was never meant to provide a robust server. There's a reason we got rid of it in 3.15.

We need timeout logic here to be able to test this change.