From 6715eb4f4f1b80a093b98aacf7f998fac876918f Mon Sep 17 00:00:00 2001 From: Marcelo Trylesinski Date: Sat, 16 Dec 2023 13:47:54 +0100 Subject: [PATCH] Deprecate `FileResponse(method=...)` parameter (#2366) --- pyproject.toml | 1 + starlette/responses.py | 10 ++++++++-- starlette/staticfiles.py | 8 ++------ tests/test_responses.py | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9909fb86a..6e7e4e12b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -88,6 +88,7 @@ filterwarnings = [ "ignore: The `allow_redirects` argument is deprecated. Use `follow_redirects` instead.:DeprecationWarning", "ignore: 'cgi' is deprecated and slated for removal in Python 3.13:DeprecationWarning", "ignore: You seem to already have a custom sys.excepthook handler installed. I'll skip installing Trio's custom handler, but this means MultiErrors will not show full tracebacks.:RuntimeWarning", + "ignore: The 'method' parameter is not used, and it will be removed.:DeprecationWarning:starlette", ] [tool.coverage.run] diff --git a/starlette/responses.py b/starlette/responses.py index bc8b23f1c..507e79b32 100644 --- a/starlette/responses.py +++ b/starlette/responses.py @@ -3,6 +3,7 @@ import os import stat import typing +import warnings from datetime import datetime from email.utils import format_datetime, formatdate from functools import partial @@ -10,6 +11,7 @@ from urllib.parse import quote import anyio +import anyio.to_thread from starlette._compat import md5_hexdigest from starlette.background import BackgroundTask @@ -280,7 +282,11 @@ def __init__( self.path = path self.status_code = status_code self.filename = filename - self.send_header_only = method is not None and method.upper() == "HEAD" + if method is not None: + warnings.warn( + "The 'method' parameter is not used, and it will be removed.", + DeprecationWarning, + ) if media_type is None: media_type = guess_type(filename or path)[0] or "text/plain" self.media_type = media_type @@ -329,7 +335,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: "headers": self.raw_headers, } ) - if self.send_header_only: + if scope["method"].upper() == "HEAD": await send({"type": "http.response.body", "body": b"", "more_body": False}) else: async with await anyio.open_file(self.path, mode="rb") as file: diff --git a/starlette/staticfiles.py b/starlette/staticfiles.py index f36d58664..30fbc11d0 100644 --- a/starlette/staticfiles.py +++ b/starlette/staticfiles.py @@ -6,6 +6,7 @@ from email.utils import parsedate import anyio +import anyio.to_thread from starlette.datastructures import URL, Headers from starlette.exceptions import HTTPException @@ -154,12 +155,7 @@ async def get_response(self, path: str, scope: Scope) -> Response: self.lookup_path, "404.html" ) if stat_result and stat.S_ISREG(stat_result.st_mode): - return FileResponse( - full_path, - stat_result=stat_result, - method=scope["method"], - status_code=404, - ) + return FileResponse(full_path, stat_result=stat_result, status_code=404) raise HTTPException(status_code=404) def lookup_path( diff --git a/tests/test_responses.py b/tests/test_responses.py index 3554ca60a..6b6d07970 100644 --- a/tests/test_responses.py +++ b/tests/test_responses.py @@ -2,12 +2,14 @@ import os import time from http.cookies import SimpleCookie +from pathlib import Path import anyio import pytest from starlette import status from starlette.background import BackgroundTask +from starlette.datastructures import Headers from starlette.requests import Request from starlette.responses import ( FileResponse, @@ -17,6 +19,7 @@ StreamingResponse, ) from starlette.testclient import TestClient +from starlette.types import Message def test_text_response(test_client_factory): @@ -244,6 +247,36 @@ async def app(scope, receive, send): assert filled_by_bg_task == "6, 7, 8, 9" +@pytest.mark.anyio +async def test_file_response_on_head_method(tmpdir: Path): + path = os.path.join(tmpdir, "xyz") + content = b"" * 1000 + with open(path, "wb") as file: + file.write(content) + + app = FileResponse(path=path, filename="example.png") + + async def receive() -> Message: # type: ignore[empty-body] + ... # pragma: no cover + + async def send(message: Message) -> None: + if message["type"] == "http.response.start": + assert message["status"] == status.HTTP_200_OK + headers = Headers(raw=message["headers"]) + assert headers["content-type"] == "image/png" + assert "content-length" in headers + assert "content-disposition" in headers + assert "last-modified" in headers + assert "etag" in headers + elif message["type"] == "http.response.body": + assert message["body"] == b"" + assert message["more_body"] is False + + # Since the TestClient drops the response body on HEAD requests, we need to test + # this directly. + await app({"type": "http", "method": "head"}, receive, send) + + def test_file_response_with_directory_raises_error(tmpdir, test_client_factory): app = FileResponse(path=tmpdir, filename="example.png") client = test_client_factory(app)