mirror of https://github.com/zulip/zulip.git
middleware: Do not consume StreamingHttpResponse.streaming_content.
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 <anders@zulip.com>
This commit is contained in:
parent
4fde4e7c0d
commit
98310f269b
|
@ -3,13 +3,13 @@ import logging
|
||||||
import tempfile
|
import tempfile
|
||||||
import time
|
import time
|
||||||
import traceback
|
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 urllib.parse import urlencode, urljoin
|
||||||
|
|
||||||
from django.conf import settings
|
from django.conf import settings
|
||||||
from django.conf.urls.i18n import is_language_prefix_patterns_used
|
from django.conf.urls.i18n import is_language_prefix_patterns_used
|
||||||
from django.db import connection
|
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.http.response import HttpResponseBase
|
||||||
from django.middleware.locale import LocaleMiddleware as DjangoLocaleMiddleware
|
from django.middleware.locale import LocaleMiddleware as DjangoLocaleMiddleware
|
||||||
from django.shortcuts import render
|
from django.shortcuts import render
|
||||||
|
@ -128,13 +128,8 @@ def write_log_line(
|
||||||
client_name: str,
|
client_name: str,
|
||||||
client_version: Optional[str] = None,
|
client_version: Optional[str] = None,
|
||||||
status_code: int = 200,
|
status_code: int = 200,
|
||||||
error_content: Optional[AnyStr] = None,
|
error_content: Optional[bytes] = None,
|
||||||
error_content_iter: Optional[Iterable[AnyStr]] = None,
|
|
||||||
) -> 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
|
time_delta = -1
|
||||||
# A time duration of -1 means the StartLogRequests middleware
|
# A time duration of -1 means the StartLogRequests middleware
|
||||||
# didn't run for some reason
|
# didn't run for some reason
|
||||||
|
@ -225,14 +220,7 @@ def write_log_line(
|
||||||
|
|
||||||
# Log some additional data whenever we return certain 40x errors
|
# Log some additional data whenever we return certain 40x errors
|
||||||
if 400 <= status_code < 500 and status_code not in [401, 404, 405]:
|
if 400 <= status_code < 500 and status_code not in [401, 404, 405]:
|
||||||
assert error_content_iter is not None
|
error_data = repr(error_content)
|
||||||
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))
|
|
||||||
if len(error_data) > 200:
|
if len(error_data) > 200:
|
||||||
error_data = "[content more than 200 characters]"
|
error_data = "[content more than 200 characters]"
|
||||||
logger.info("status=%3d, data=%s, uid=%s", status_code, error_data, requestor_for_logs)
|
logger.info("status=%3d, data=%s, uid=%s", status_code, error_data, requestor_for_logs)
|
||||||
|
@ -360,9 +348,6 @@ class LogRequests(MiddlewareMixin):
|
||||||
else:
|
else:
|
||||||
requestor_for_logs = "unauth@{}".format(get_subdomain(request) or "root")
|
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
|
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
|
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,
|
client_version=request_notes.client_version,
|
||||||
status_code=response.status_code,
|
status_code=response.status_code,
|
||||||
error_content=content,
|
error_content=content,
|
||||||
error_content_iter=content_iter,
|
|
||||||
)
|
)
|
||||||
return response
|
return response
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue