gh-150107: adding hasattr check on offset to include if available#150142
gh-150107: adding hasattr check on offset to include if available#150142grantlouisherman wants to merge 18 commits into
Conversation
…able Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Hey @1st1 does this need a news blurb? IT seems pretty small |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
|
Also I cant add the tags but this needs to be back ported to 3.12 and 3.13 |
|
|
Sounds good @picnixz Ill add news now |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
|
You can add the following def check_sock_sendfile_offset(self, data, offset, force_fallback=False):
sock, proto = self.prepare_socksendfile()
with tempfile.TemporaryFile() as f:
f.write(data)
f.flush()
self.assertEqual(f.tell(), len(data))
if force_fallback:
async def _sock_sendfile_fail(sock, file, offset, count):
raise asyncio.exceptions.SendfileNotAvailableError()
with support.swap_attr(self.loop, '_sock_sendfile_native', _sock_sendfile_fail):
ret = self.run_loop(self.loop.sock_sendfile(sock, f, offset, None))
else:
ret = self.run_loop(self.loop.sock_sendfile(sock, f, offset, None))
self.assertEqual(f.tell(), len(data))
sock.close()
self.run_loop(proto.wait_closed())
self.assertEqual(ret, len(data) - offset)
def test_sock_sendfile_offset(self):
data = b'abcdef'
for offset in (0, len(data) // 2, len(data)):
for force_fallback in (False, True):
with self.subTest(offset=offset, force_fallback=force_fallback):
self.check_sock_sendfile_offset(data, offset, force_fallback) |
|
I also wrote a draft test for def check_sendfile_offset(self, offset, fallback):
srv_proto, cli_proto = self.prepare_sendfile()
self.file.seek(123)
coro = self.loop.sendfile(cli_proto.transport, self.file, offset, fallback=fallback)
ret = self.run_loop(coro)
cli_proto.transport.close()
self.run_loop(srv_proto.done)
self.assertEqual(ret, len(self.DATA) - offset)
self.assertEqual(srv_proto.nbytes, len(self.DATA) - offset)
self.assertEqual(srv_proto.data, self.DATA[offset:])
#self.assertEqual(self.file.tell(), len(self.DATA))
def test_sendfile_offset(self):
for offset in (0, len(self.DATA) // 2, len(self.DATA)):
for fallback in (False, True):
with self.subTest(offset=offset, fallback=fallback):
self.check_sendfile_offset(offset, fallback) |
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
|
@vstinner I just c/p your tests, I was going to try and use whatever the original repro was but I wasnt sure how to make that into test form so thanks for the snippet~ |
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
Signed-off-by: grantlouisherman <grantlouisherman041@gmail.com>
| offset_high = (offset >> 32) & 0xffff_ffff | ||
| # TransmitFile ignores OVERLAPPED.Offset for handles not opened with | ||
| # FILE_FLAG_OVERLAPPED, so seek the CRT file pointer to match. | ||
| file.seek(offset) |
There was a problem hiding this comment.
@vstinner this was an AI debugged and verified by me after the windows unit tests were crashing. Apparently the issue is that this function was essentially ignoring the offset being passed because we were not passing in OVERLAPPED.Offset. So test_sock_sendfile_offset was crashing because essentially we write 6 bytes and point to EOF. Then pass in the offset of 0 the rewind however because TransmitFile was ignoring the overlap the tests failed with an EOF error. There was a similar error reported but never got merged #112337
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.