Skip to content

Commit fb2221c

Browse files
committed
Fix if-range handling and its unit tests
The value used for conditional range requests should be updated when the download is restarted (not *resumed*). The unit tests testing conditional range requests were broken, using code 206 for the non-matching responses. This violates the HTTP spec and fools the resumption logic into thinking the conditional range request was fulfilled (when it wasn't!). The test cases were fixed to use code 200 as expected and extended to ensure the content key is updated across a chain of non-matching repsonses (which is borderline impossible, but hey it did catch my eye so why not fix it).
1 parent 938ec08 commit fb2221c

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/pip/_internal/network/download.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ def _process_response(self, download: _FileDownload, resp: Response) -> None:
216216
logger.warning("Connection timed out while downloading.")
217217

218218
def _attempt_resumes_or_redownloads(
219-
self, download: _FileDownload, resp: Response
219+
self, download: _FileDownload, first_resp: Response
220220
) -> None:
221221
"""Attempt to resume/restart the download if connection was dropped."""
222222

@@ -231,14 +231,15 @@ def _attempt_resumes_or_redownloads(
231231
)
232232

233233
try:
234-
resume_resp = self._http_get_resume(download, should_match=resp)
234+
resume_resp = self._http_get_resume(download, should_match=first_resp)
235235
# Fallback: if the server responded with 200 (i.e., the file has
236236
# since been modified or range requests are unsupported) or any
237237
# other unexpected status, restart the download from the beginning.
238238
must_restart = resume_resp.status_code != HTTPStatus.PARTIAL_CONTENT
239239
if must_restart:
240240
download.reset_file()
241241
download.size = _get_http_response_size(resume_resp)
242+
first_resp = resume_resp
242243

243244
self._process_response(download, resume_resp)
244245
except (ConnectionError, ReadTimeoutError, OSError):

tests/unit/test_network_download.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,9 @@ def test_parse_content_disposition(
229229
[(24, None), (36, None)],
230230
b"new-0cfa7e9d-1868-4dd7-9fb3-f2561d5dfd89",
231231
),
232-
# The downloader should fail after n resume_retries attempts.
232+
# The downloader should fail after N resume_retries attempts.
233233
# This prevents the downloader from getting stuck if the connection
234-
# is unstable and the server doesn't not support range requests.
234+
# is unstable and the server does NOT support range requests.
235235
(
236236
1,
237237
[
@@ -241,7 +241,7 @@ def test_parse_content_disposition(
241241
[(24, None)],
242242
None,
243243
),
244-
# The downloader should use If-Range header to make the range
244+
# The downloader should use the If-Range header to make the range
245245
# request conditional if it is possible to check for modifications
246246
# (e.g. if we know the creation time of the initial response).
247247
(
@@ -255,17 +255,28 @@ def test_parse_content_disposition(
255255
200,
256256
b"0cfa7e9d-1868-4dd7-9fb3-",
257257
),
258+
(
259+
{
260+
"content-length": "42",
261+
"last-modified": "Wed, 21 Oct 2015 07:30:00 GMT",
262+
},
263+
200,
264+
b"new-0cfa7e9d-1868-4dd7-9fb3-f2561d5dfd89",
265+
),
258266
(
259267
{
260268
"content-length": "12",
261269
"last-modified": "Wed, 21 Oct 2015 07:54:00 GMT",
262270
},
263-
206,
271+
200,
264272
b"f2561d5dfd89",
265273
),
266274
],
267-
[(24, "Wed, 21 Oct 2015 07:28:00 GMT")],
268-
b"0cfa7e9d-1868-4dd7-9fb3-f2561d5dfd89",
275+
[
276+
(24, "Wed, 21 Oct 2015 07:28:00 GMT"),
277+
(40, "Wed, 21 Oct 2015 07:30:00 GMT"),
278+
],
279+
b"f2561d5dfd89",
269280
),
270281
# ETag is preferred over Last-Modified for the If-Range condition.
271282
(
@@ -286,12 +297,12 @@ def test_parse_content_disposition(
286297
"last-modified": "Wed, 21 Oct 2015 07:54:00 GMT",
287298
"etag": '"33a64df551425fcc55e4d42a148795d9f25f89d4"',
288299
},
289-
206,
300+
200,
290301
b"f2561d5dfd89",
291302
),
292303
],
293304
[(24, '"33a64df551425fcc55e4d42a148795d9f25f89d4"')],
294-
b"0cfa7e9d-1868-4dd7-9fb3-f2561d5dfd89",
305+
b"f2561d5dfd89",
295306
),
296307
],
297308
)

0 commit comments

Comments
 (0)