Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Dec 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.",
Copy link
Member Author

@Kludex Kludex Dec 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method parameter becomes a no-op with this PR, and it's "scheduled" to be removed in 1.0.

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