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

Implement file handling independent of aiofiles, add support for file-like objects #480

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ For a more complete example, see [encode/starlette-example](https://github.com/e
Starlette does not have any hard dependencies, but the following are optional:

* [`requests`][requests] - Required if you want to use the `TestClient`.
* [`aiofiles`][aiofiles] - Required if you want to use `FileResponse` or `StaticFiles`.
* [`jinja2`][jinja2] - Required if you want to use `Jinja2Templates`.
* [`python-multipart`][python-multipart] - Required if you want to support form parsing, with `request.form()`.
* [`itsdangerous`][itsdangerous] - Required for `SessionMiddleware` support.
Expand Down Expand Up @@ -168,7 +167,6 @@ gunicorn -k uvicorn.workers.UvicornH11Worker ...
<p align="center"><i>Starlette is <a href="https://github.com/encode/starlette/blob/master/LICENSE.md">BSD licensed</a> code. Designed & built in Brighton, England.</i></p>

[requests]: http://docs.python-requests.org/en/master/
[aiofiles]: https://github.com/Tinche/aiofiles
[jinja2]: http://jinja.pocoo.org/
[python-multipart]: https://andrew-d.github.io/python-multipart/
[graphene]: https://graphene-python.org/
Expand Down
2 changes: 0 additions & 2 deletions docs/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ For a more complete example, [see here](https://github.com/encode/starlette-exam
Starlette does not have any hard dependencies, but the following are optional:

* [`requests`][requests] - Required if you want to use the `TestClient`.
* [`aiofiles`][aiofiles] - Required if you want to use `FileResponse` or `StaticFiles`.
* [`jinja2`][jinja2] - Required if you want to use `Jinja2Templates`.
* [`python-multipart`][python-multipart] - Required if you want to support form parsing, with `request.form()`.
* [`itsdangerous`][itsdangerous] - Required for `SessionMiddleware` support.
Expand Down Expand Up @@ -164,7 +163,6 @@ gunicorn -k uvicorn.workers.UvicornH11Worker ...
<p align="center"><i>Starlette is <a href="https://github.com/encode/starlette/blob/master/LICENSE.md">BSD licensed</a> code. Designed & built in Brighton, England.</i></p>

[requests]: http://docs.python-requests.org/en/master/
[aiofiles]: https://github.com/Tinche/aiofiles
[jinja2]: http://jinja.pocoo.org/
[python-multipart]: https://andrew-d.github.io/python-multipart/
[graphene]: https://graphene-python.org/
Expand Down
8 changes: 4 additions & 4 deletions docs/responses.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,12 @@ Asynchronously streams a file as the response.

Takes a different set of arguments to instantiate than the other response types:

* `path` - The filepath to the file to stream.
* `file_or_path` - Can be a file path as a `str`, or a `pathlib.Path`. Can also be a [file-like object](https://docs.python.org/3/glossary.html#term-file-like-object) like those returned by `open(some_path, mode="rb")`.
* `headers` - Any custom headers to include, as a dictionary.
* `media_type` - A string giving the media type. If unset, the filename or path will be used to infer a media type.
* `filename` - If set, this will be included in the response `Content-Disposition`.
* `media_type` - A string giving the media type. If unset, the filename or path will be used to infer a media type (if `file_or_path` is a path).
* `filename` - If set, this will be included in the response `Content-Disposition`. For example, instructing the browser to download the file instead of displaying it.

File responses will include appropriate `Content-Length`, `Last-Modified` and `ETag` headers.
File responses will include appropriate `Content-Length`, `Last-Modified` and `ETag` headers if `file_or_path` was a path.

```python
from starlette.responses import FileResponse
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Optionals
aiofiles
graphene
itsdangerous
jinja2
Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ def get_packages(package):
data_files=[("", ["LICENSE.md"])],
extras_require={
"full": [
"aiofiles",
"asyncpg",
"graphene",
"itsdangerous",
Expand Down
63 changes: 40 additions & 23 deletions starlette/responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,18 @@
import inspect
import json
import os
import pathlib
import stat
import typing
from email.utils import formatdate
from mimetypes import guess_type
from urllib.parse import quote_plus

from starlette.background import BackgroundTask
from starlette.concurrency import iterate_in_threadpool
from starlette.concurrency import iterate_in_threadpool, run_in_threadpool
from starlette.datastructures import URL, MutableHeaders
from starlette.types import Receive, Scope, Send

try:
import aiofiles
from aiofiles.os import stat as aio_stat
except ImportError: # pragma: nocover
aiofiles = None # type: ignore
aio_stat = None # type: ignore

try:
import ujson
except ImportError: # pragma: nocover
Expand Down Expand Up @@ -209,7 +203,7 @@ class FileResponse(Response):

def __init__(
self,
path: str,
file_or_path: typing.Union[str, pathlib.Path, typing.IO[str], typing.IO[bytes]],
status_code: int = 200,
headers: dict = None,
media_type: str = None,
Expand All @@ -218,13 +212,21 @@ def __init__(
stat_result: os.stat_result = None,
method: str = None,
) -> None:
assert aiofiles is not None, "'aiofiles' must be installed to use FileResponse"
self.path = path
# self.file_or_path: typing.Union[str, pathlib.Path, typing.IO[str], typing.IO[bytes]] = file_or_path
self.file_or_path = file_or_path
self.is_readable = hasattr(file_or_path, "read")
self.status_code = status_code
self.filename = filename
self.send_header_only = method is not None and method.upper() == "HEAD"
if media_type is None:
media_type = guess_type(filename or path)[0] or "text/plain"
if filename:
media_type = guess_type(filename)[0]
elif not self.is_readable:
media_type = guess_type(str(file_or_path))[0]
else:
# Default to generic media type "application/octet-stream", ref:
# https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types
media_type = "application/octet-stream"
self.media_type = media_type
self.background = background
self.init_headers(headers)
Expand All @@ -247,15 +249,20 @@ def set_stat_headers(self, stat_result: os.stat_result) -> None:

async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if self.stat_result is None:
try:
stat_result = await aio_stat(self.path)
self.set_stat_headers(stat_result)
except FileNotFoundError:
raise RuntimeError(f"File at path {self.path} does not exist.")
else:
mode = stat_result.st_mode
if not stat.S_ISREG(mode):
raise RuntimeError(f"File at path {self.path} is not a file.")
if not self.is_readable:
try:
stat_result = await run_in_threadpool(os.stat, self.file_or_path)
self.set_stat_headers(stat_result)
except FileNotFoundError:
raise RuntimeError(
f"File at path {self.file_or_path} does not exist."
)
else:
mode = stat_result.st_mode
if not stat.S_ISREG(mode):
raise RuntimeError(
f"File at path {self.file_or_path} is not a file."
)
await send(
{
"type": "http.response.start",
Expand All @@ -266,10 +273,16 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
if self.send_header_only:
await send({"type": "http.response.body"})
else:
async with aiofiles.open(self.path, mode="rb") as file:
if self.is_readable:
file = typing.cast(
typing.Union[typing.IO[str], typing.IO[bytes]], self.file_or_path
)
else:
file = await run_in_threadpool(open, self.file_or_path, mode="rb")
try:
more_body = True
while more_body:
chunk = await file.read(self.chunk_size)
chunk = await run_in_threadpool(file.read, self.chunk_size)
more_body = len(chunk) == self.chunk_size
await send(
{
Expand All @@ -278,5 +291,9 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
"more_body": more_body,
}
)
except Exception as e:
if hasattr(file, "close"):
await run_in_threadpool(file.close)
raise RuntimeError(f"Error processing file in FileResponse: {e}")
if self.background is not None:
await self.background()
7 changes: 3 additions & 4 deletions starlette/staticfiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
import typing
from email.utils import parsedate

from aiofiles.os import stat as aio_stat

from starlette.concurrency import run_in_threadpool
from starlette.datastructures import URL, Headers
from starlette.responses import (
FileResponse,
Expand Down Expand Up @@ -149,7 +148,7 @@ async def lookup_path(
for directory in self.all_directories:
full_path = os.path.join(directory, path)
try:
stat_result = await aio_stat(full_path)
stat_result = await run_in_threadpool(os.stat, full_path)
return (full_path, stat_result)
except FileNotFoundError:
pass
Expand Down Expand Up @@ -182,7 +181,7 @@ async def check_config(self) -> None:
return

try:
stat_result = await aio_stat(self.directory)
stat_result = await run_in_threadpool(os.stat, self.directory)
except FileNotFoundError:
raise RuntimeError(
f"StaticFiles directory '{self.directory}' does not exist."
Expand Down
43 changes: 40 additions & 3 deletions tests/test_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ async def numbers_for_cleanup(start=1, stop=5):

async def app(scope, receive, send):
response = FileResponse(
path=path, filename="example.png", background=cleanup_task
file_or_path=path, filename="example.png", background=cleanup_task
)
await response(scope, receive, send)

Expand All @@ -174,8 +174,30 @@ async def app(scope, receive, send):
assert filled_by_bg_task == "6, 7, 8, 9"


def test_file_like_response(tmpdir):
path = os.path.join(tmpdir, "xyz")
content = b"<file content>" * 1000
with open(path, "wb") as file:
file.write(content)

async def app(scope, receive, send):
with open(path, "rb") as file_like:
response = FileResponse(file_or_path=file_like)
await response(scope, receive, send)

client = TestClient(app)
response = client.get("/")
assert response.status_code == status.HTTP_200_OK
assert response.content == content
assert response.headers["content-type"] == "application/octet-stream"
assert "content-disposition" not in response.headers
assert "content-length" not in response.headers
assert "last-modified" not in response.headers
assert "etag" not in response.headers


def test_file_response_with_directory_raises_error(tmpdir):
app = FileResponse(path=tmpdir, filename="example.png")
app = FileResponse(file_or_path=str(tmpdir), filename="example.png")
client = TestClient(app)
with pytest.raises(RuntimeError) as exc:
client.get("/")
Expand All @@ -184,13 +206,28 @@ def test_file_response_with_directory_raises_error(tmpdir):

def test_file_response_with_missing_file_raises_error(tmpdir):
path = os.path.join(tmpdir, "404.txt")
app = FileResponse(path=path, filename="404.txt")
app = FileResponse(file_or_path=path, filename="404.txt")
client = TestClient(app)
with pytest.raises(RuntimeError) as exc:
client.get("/")
assert "does not exist" in str(exc)


def test_file_response_reading_error_raises_error():
class FakeFile:
def read(self):
raise ValueError("No content here") # pragma: no cover

def close(self):
pass

app = FileResponse(file_or_path=FakeFile())
client = TestClient(app)
with pytest.raises(RuntimeError) as exc:
client.get("/")
assert "Error processing file in FileResponse" in str(exc)


def test_set_cookie():
async def app(scope, receive, send):
response = Response("Hello, world!", media_type="text/plain")
Expand Down