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

Abierto
  • ¿Algo no está bien?
#6,512 6 comentarios 4 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
35/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Estancado
Stack tecnológico
python
Área
api, networking

Línea de trabajo

Empieza en src/requests/adapters.py y src/requests/models.py, y después lee las pruebas de respuesta de bajo nivel existentes en tests/test_lowlevel.py. Ejecuta la reproducción de la respuesta truncada y compárala con test_retry_truncated_response; se considera terminado cuando una solicitud que no es de streaming se reintenta según la política de HTTPAdapter y devuelve el contenido completo esperado sin filtrar excepciones de urllib3.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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()
Lenguaje dominante
Python
Estrellas
54.7k
Forks
10.8k
Merge medio
18 min
PR fusionados (30 d)
1

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de psf/requests

Todos los issues de psf/requests

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.