Deprecate FileResponse(method=...) parameter (#2366)

Kludex · web-flow · commit 6715eb4f4f1b · 2023-12-16T12:47:54.000Z
diff --git 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
@@ -3,13 +3,15 @@
 import os
 import stat
 import typing
+import warnings
 from datetime import datetime
 from email.utils import format_datetime, formatdate
 from functools import partial
 from mimetypes import guess_type
 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
@@ -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
@@ -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"<file content>" * 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)

Original file line number	Diff line number	Diff line change
`@@ -88,6 +88,7 @@ filterwarnings = [`
`88`	`88`	"ignore: The `allow_redirects` argument is deprecated. Use `follow_redirects` instead.:DeprecationWarning",
`89`	`89`	`"ignore: 'cgi' is deprecated and slated for removal in Python 3.13:DeprecationWarning",`
`90`	`90`	`"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",`
	`91`	`+ "ignore: The 'method' parameter is not used, and it will be removed.:DeprecationWarning:starlette",`
`91`	`92`	`]`
`92`	`93`
`93`	`94`	`[tool.coverage.run]`