Requests are not retried when received body length is shorter than Content-Length

Aperta
  • Qualcosa non va?
#6,512 6 commenti 4 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
35/100
Tipo di issue
Bug
Chiarezza
Abbastanza chiara
Stato di attività
Ferma
Stack tecnologico
python
Ambito
api, networking

Direzione di ricerca

Inizia da src/requests/adapters.py e src/requests/models.py, quindi leggi i test esistenti delle risposte di basso livello in tests/test_lowlevel.py. Esegui la riproduzione della risposta troncata e confrontala con test_retry_truncated_response; il lavoro è completato quando una richiesta non in streaming viene ritentata secondo la policy di HTTPAdapter e restituisce il contenuto completo previsto senza far trapelare eccezioni di urllib3.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

When a server sends less bytes than indicated by Content-Length, we get a ChunkedEncodingError instead of retrying the request.

urllib3 supports retrying requests in this situation by setting preload_content=True. When a user specifies stream=True, obviously, all bets are off: the response cannot be preloaded and therefore the request cannot be retried. However, even when stream=False, the response is still not preloaded and therefore the urllib3 retry mechanism in this situation is bypassed.

As a background to this issue, I've been investigating rare failures in my CI builds during pip install. I believe this issue to be the proximate cause: pip makes some requests to PyPI, with stream=False and retries configured but still fails.

In the current version of pip (which has an out of date urllib3 package), pip falls victim to https://github.com/psf/requests/issues/4956 and fails to parse the PyPI metadata with a JSONDecodeError. Upgrading pip's urllib3 version results in a ChunkedEncodingError as below.

Expected Result

The request is retried according to the specified retry policy.

Actual Result

requests.exceptions.ChunkedEncodingError: ('Connection broken: IncompleteRead(10 bytes read, 26227 more expected)', IncompleteRead(10 bytes read, 26227 more expected))

Because the response is not preloaded, urllib3 cannot retry the request, and requests has no retry functionality of its own.

Reproduction Steps

import requests
from requests.adapters import HTTPAdapter

s = requests.Session()
s.mount("http://", HTTPAdapter(max_retries=5))

r = s.get('http://127.0.0.1:5000/test', stream=False)

I'm using an intentionally broken local server for testing. See here for an example.

System Information

$ python -m requests.help

{
  "chardet": {
    "version": null
  },
  "charset_normalizer": {
    "version": "3.2.0"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "3.4"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.11.4"
  },
  "platform": {
    "release": "6.4.11-100.fc37.x86_64",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.31.0"
  },
  "system_ssl": {
    "version": "30000090"
  },
  "urllib3": {
    "version": "2.0.4"
  },
  "using_charset_normalizer": true,
  "using_pyopenssl": false
}

Proposed Patch

I have a proposed patch which I believe fixes this problem. Unfortunately, my patch breaks a bunch of the tests (and probably also breaks backwards compatibility, in particular, this patch causes requests to start leaking urllib3 exceptions). On the off chance it's useful in coming up with a proper fix, here it is:

diff --git a/src/requests/adapters.py b/src/requests/adapters.py
index eb240fa9..ce01c2a5 100644
--- a/src/requests/adapters.py
+++ b/src/requests/adapters.py
@@ -489,8 +489,8 @@ class HTTPAdapter(BaseAdapter):
                 headers=request.headers,
                 redirect=False,
                 assert_same_host=False,
-                preload_content=False,
-                decode_content=False,
+                preload_content=not stream,
+                decode_content=not stream,
                 retries=self.max_retries,
                 timeout=timeout,
                 chunked=chunked,
diff --git a/src/requests/models.py b/src/requests/models.py
index 44556394..f43f1bf8 100644
--- a/src/requests/models.py
+++ b/src/requests/models.py
@@ -893,6 +893,8 @@ class Response:
 
             if self.status_code == 0 or self.raw is None:
                 self._content = None
+            elif getattr(self.raw, "data", None) is not None:
+                self._content = self.raw.data
             else:
                 self._content = b"".join(self.iter_content(CONTENT_CHUNK_SIZE)) or b""
 
diff --git a/tests/test_lowlevel.py b/tests/test_lowlevel.py
index 859d07e8..39a1175e 100644
--- a/tests/test_lowlevel.py
+++ b/tests/test_lowlevel.py
@@ -4,6 +4,7 @@ import pytest
 from tests.testserver.server import Server, consume_socket_content
 
 import requests
+from requests.adapters import HTTPAdapter
 from requests.compat import JSONDecodeError
 
 from .utils import override_environ
@@ -426,3 +427,33 @@ def test_json_decode_compatibility_for_alt_utf_encodings():
     assert isinstance(excinfo.value, requests.exceptions.RequestException)
     assert isinstance(excinfo.value, JSONDecodeError)
     assert r.text not in str(excinfo.value)
+
+
+def test_retry_truncated_response():
+    data = b"truncated before retry"
+    response_lengths = [len(data), 9]
+
+    def retry_handler(sock):
+        request_content = consume_socket_content(sock, timeout=0.5)
+
+        response = (
+            b"HTTP/1.1 200 OK\r\n"
+            b"Content-Length: %d\r\n\r\n"
+            b"%s"
+        ) % (len(data), data[:response_lengths.pop()])
+        sock.send(response)
+
+        return request_content
+
+    close_server = threading.Event()
+    server = Server(retry_handler, wait_to_close_event=close_server, requests_to_handle=2)
+
+    s = requests.Session()
+    s.mount("http://", HTTPAdapter(max_retries=2))
+
+    with server as (host, port):
+        url = f"http://{host}:{port}/"
+        r = s.get(url, stream=False)
+        assert r.status_code == 200
+        assert r.content == data
+        close_server.set()
Lingua principale
Python
Stelle
54.7k
Fork
10.8k
Merge medio
18m
PR unite (30g)
1

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di psf/requests

Tutte le issue di psf/requests

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.