From 98310f269bf32cebae57a5b2eb5c636ffb278468 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Fri, 16 Jun 2023 16:21:08 -0700 Subject: [PATCH] middleware: Do not consume StreamingHttpResponse.streaming_content. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit streaming_content is an iterator. Consuming it within middleware prevents it from being sent to the browser. https://docs.djangoproject.com/en/4.2/ref/request-response/#streaminghttpresponse-objects “The StreamingHttpResponse … has no content attribute. Instead, it has a streaming_content attribute. This can be used in middleware to wrap the response iterable, but should not be consumed.” Signed-off-by: Anders Kaseorg --- zerver/middleware.py | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/zerver/middleware.py b/zerver/middleware.py index 813e166a80..f2d5cbca76 100644 --- a/zerver/middleware.py +++ b/zerver/middleware.py @@ -3,13 +3,13 @@ import logging import tempfile import time import traceback -from typing import Any, AnyStr, Callable, Dict, Iterable, List, MutableMapping, Optional, Tuple +from typing import Any, Callable, Dict, List, MutableMapping, Optional, Tuple from urllib.parse import urlencode, urljoin from django.conf import settings from django.conf.urls.i18n import is_language_prefix_patterns_used from django.db import connection -from django.http import HttpRequest, HttpResponse, HttpResponseRedirect, StreamingHttpResponse +from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.http.response import HttpResponseBase from django.middleware.locale import LocaleMiddleware as DjangoLocaleMiddleware from django.shortcuts import render @@ -128,13 +128,8 @@ def write_log_line( client_name: str, client_version: Optional[str] = None, status_code: int = 200, - error_content: Optional[AnyStr] = None, - error_content_iter: Optional[Iterable[AnyStr]] = None, + error_content: Optional[bytes] = None, ) -> None: - assert error_content is None or error_content_iter is None - if error_content is not None: - error_content_iter = (error_content,) - time_delta = -1 # A time duration of -1 means the StartLogRequests middleware # didn't run for some reason @@ -225,14 +220,7 @@ def write_log_line( # Log some additional data whenever we return certain 40x errors if 400 <= status_code < 500 and status_code not in [401, 404, 405]: - assert error_content_iter is not None - error_content_list = list(error_content_iter) - if not error_content_list: - error_data = "" - elif isinstance(error_content_list[0], str): - error_data = "".join(error_content_list) - elif isinstance(error_content_list[0], bytes): - error_data = repr(b"".join(error_content_list)) + error_data = repr(error_content) if len(error_data) > 200: error_data = "[content more than 200 characters]" logger.info("status=%3d, data=%s, uid=%s", status_code, error_data, requestor_for_logs) @@ -360,9 +348,6 @@ class LogRequests(MiddlewareMixin): else: requestor_for_logs = "unauth@{}".format(get_subdomain(request) or "root") - content_iter = ( - response.streaming_content if isinstance(response, StreamingHttpResponse) else None - ) content = response.content if isinstance(response, HttpResponse) else None assert request_notes.client_name is not None and request_notes.log_data is not None @@ -377,7 +362,6 @@ class LogRequests(MiddlewareMixin): client_version=request_notes.client_version, status_code=response.status_code, error_content=content, - error_content_iter=content_iter, ) return response