Skip to content

Commit

Permalink
Deprecate FileResponse(method=...) parameter (#2366)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kludex authored Dec 16, 2023
1 parent 1ed1737 commit 6715eb4
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 8 deletions.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
10 changes: 8 additions & 2 deletions starlette/responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 2 additions & 6 deletions starlette/staticfiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
33 changes: 33 additions & 0 deletions tests/test_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -17,6 +19,7 @@
StreamingResponse,
)
from starlette.testclient import TestClient
from starlette.types import Message


def test_text_response(test_client_factory):
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 6715eb4

Please sign in to comment.