gh-150107: Fix asyncio sendfile fallback ignoring non-zero offset#150270
gh-150107: Fix asyncio sendfile fallback ignoring non-zero offset#150270grantlouisherman wants to merge 3 commits into
Conversation
|
We should also fix the issue in |
|
Sure @vstinner I must have missed that comment. |
688d182 to
3b33313
Compare
|
So in @vstinner from doing some digging and playing around with this I essentially updated the unix event to always update the file pointers based on the current offset. I think the previous guard was buggy, obviously exposed by the test you drafted, where if total sent returned 0 which was the case when we were EOF a file pointer wouldnt get updated because total sent would == 0. Let me know your thoughts |
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
dc70850 to
2a884c8
Compare
|
@1st1 is there anything Im missing from this? |
Original Bug Report:
Bug report
Bug description:
BaseEventLoop._sock_sendfile_fallback(https://github.com/python/cpython/blob/main/Lib/asyncio/base_events.py#L971) erroneously doesn't seek when passed the offset 0.This check in the function doesn't call
file.seekif offset is falsey. If the file in question has a file pointer that is not currently at the 0 offset then this will erroneously send from the current offset and not the start of the file as desired. This can happen whensendfileis called with the destination socket is an SSL socket.Repro code (generated by Claude), because of needing to create an SSL socket it's a bit involved - including making a self signed cert with the openssl command.
The relevant lines are the line
f.seek(len(CONTENT))and the linesent = await loop.sendfile(writer.transport, f, offset=0). Most of the rest is setup and teardown.CPython versions tested on:
3.12, 3.13
Operating systems tested on:
Linux
Fix
I think what I did was correct or understood the issue, but essentially as far as I could understand
0is always falsey so if offset was ever set to zero than the file.seek wasent happening even though 0 offset is a legitimate offset. I simply just did an instance on offset to make sure it is an int and then the f.seek goes through. I validated with the repro command and passing unit tests.