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

fix: Escape URLs when files are served by nginx #1224

Merged
merged 1 commit into from
Oct 6, 2024
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
36 changes: 11 additions & 25 deletions backend/endpoints/rom.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from streaming_form_data.targets import FileTarget, NullTarget
from utils.filesystem import sanitize_filename
from utils.hashing import crc32_to_hex
from utils.nginx import ZipContentLine, ZipResponse
from utils.nginx import FileRedirectResponse, ZipContentLine, ZipResponse
from utils.router import APIRouter

router = APIRouter()
Expand Down Expand Up @@ -197,21 +197,14 @@ async def head_rom_content(
},
)

return Response(
media_type="application/octet-stream",
headers={
"Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"',
"X-Accel-Redirect": f"/library/{rom.full_path}",
},
return FileRedirectResponse(
download_path=Path(f"/library/{rom.full_path}"),
filename=rom.file_name,
)

if len(files_to_check) == 1:
return Response(
media_type="application/octet-stream",
headers={
"Content-Disposition": f'attachment; filename="{quote(files_to_check[0])}"',
"X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_check[0]}",
},
return FileRedirectResponse(
download_path=Path(f"/library/{rom.full_path}/{files_to_check[0]}"),
)

return Response(
Expand Down Expand Up @@ -256,21 +249,14 @@ async def get_rom_content(
files_to_download = sorted(files or [r["filename"] for r in rom.files])

if not rom.multi:
return Response(
media_type="application/octet-stream",
headers={
"Content-Disposition": f'attachment; filename="{quote(rom.file_name)}"',
"X-Accel-Redirect": f"/library/{rom.full_path}",
},
return FileRedirectResponse(
download_path=Path(f"/library/{rom.full_path}"),
filename=rom.file_name,
)

if len(files_to_download) == 1:
return Response(
media_type="application/octet-stream",
headers={
"Content-Disposition": f'attachment; filename="{quote(files_to_download[0])}"',
"X-Accel-Redirect": f"/library/{rom.full_path}/{files_to_download[0]}",
},
return FileRedirectResponse(
download_path=Path(f"/library/{rom.full_path}/{files_to_download[0]}"),
)

content_lines = [
Expand Down
5 changes: 5 additions & 0 deletions backend/utils/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
from collections.abc import Iterator
from pathlib import Path

from anyio import Path as AnyIOPath

# Type alias for a path that can be either a `pathlib.Path` or an `anyio.Path`.
AnyPath = Path | AnyIOPath


def iter_files(path: str, recursive: bool = False) -> Iterator[tuple[Path, str]]:
"""List files in a directory.
Expand Down
26 changes: 26 additions & 0 deletions backend/utils/nginx.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import dataclasses
from collections.abc import Collection
from typing import Any
from urllib.parse import quote

from fastapi.responses import Response
from utils.filesystem import AnyPath


@dataclasses.dataclass(frozen=True)
Expand Down Expand Up @@ -47,3 +49,27 @@ def __init__(
)

super().__init__(**kwargs)


class FileRedirectResponse(Response):
"""Response class for serving a file download by using the X-Accel-Redirect header."""

def __init__(
self, *, download_path: AnyPath, filename: str | None = None, **kwargs: Any
):
"""
Arguments:
- download_path: Path to the file to be served.
- filename: Name of the file to be served. If not provided, the file name from the
download_path is used.
"""
media_type = kwargs.pop("media_type", "application/octet-stream")
filename = filename or download_path.name
kwargs.setdefault("headers", {}).update(
{
"Content-Disposition": f'attachment; filename="{quote(filename)}"',
"X-Accel-Redirect": quote(str(download_path)),
}
)

super().__init__(media_type=media_type, **kwargs)
Loading