Repository navigation
Conversation
Wrap seekable file-like Request.data in a re-iterator before Basic and Digest authentication retries so the POST body is resent after a 401. Non-seekable streams raise ValueError instead of sending an empty body.
Documentation build overview
165 files changed ·
|
|
@gpshead could you please review this ( I am a new contributor, trying to find best ways to help) ? |
|
@serhiy-storchaka , would you like to review this? |
ambv
left a comment
ambv
left a comment
There was a problem hiding this comment.
Instead of wrapping req.data, can't we just record where the body starts and seek the original object back there before retrying? That solves the things I'm pointing out in the comments.
| self._blocksize = blocksize | ||
|
|
||
| def __iter__(self): | ||
| self._fileobj.seek(0) |
There was a problem hiding this comment.
The retry rewinds to offset 0, not to where the body started, so it can send different bytes. Try:
f = BytesIO(b"SKIPME|payload"); f.seek(7)
This is a regression compared to sending an empty response because the latter usually gets rejected.
| if not getattr(data, 'seekable', lambda: hasattr(data, 'seek'))(): | ||
| raise ValueError("seekable file-like Request.data is required when " | ||
| "the request may be retried (such as HTTP authentication retries).") | ||
| request.data = _AuthBodyReiterator(data) |
There was a problem hiding this comment.
Swapping req.data drops the caller's Content-Length and quietly switches the retry to chunked encoding. Chunked encoding is the default, but that means when callers use Content-Length, they mean it, because for instance the server won't take chunked uploads.
| def __iter__(self): | ||
| self._fileobj.seek(0) | ||
| while chunk := self._fileobj.read(self._blocksize): | ||
| yield chunk |
There was a problem hiding this comment.
Yielding raw chunks for files open in text-mode can fail. That's not what http.client is doing here.
|
When you're done making the requested changes, leave the comment: |
Yup, sounds good. For the non-seekable body, we will not do a retry. We can retry the 401, so that sender and send with the fresh stream. |
Fix long-standing bug where HTTP Basic/Digest auth retries send an empty POST body when
Request.datais a consumed file-like object (e.g.mmap.mmap, open file,BytesIO).Fixes gh-49288.