mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 03:54:17 +02:00
fix(http): RFC 6266-encode Content-Disposition so non-ASCII filenames don't crash response (issue #1245)
Reported by @1000Delta. The printer file download (and three sibling endpoints) raised UnicodeEncodeError: 'latin-1' codec can't encode characters... on any filename outside U+0000..U+00FF (Chinese, Japanese, Arabic, accented Latin), because the route pushed `filename` straight into Content-Disposition: attachment; filename="...". Starlette/uvicorn encodes response headers as latin-1, so the assignment crashed at write-time. New backend/app/utils/http.py::build_content_disposition emits both an ASCII-stripped legacy filename="..." fallback and an RFC 5987 filename*=UTF-8''<percent-encoded> parameter. Every modern browser prefers the *= form, so the original Unicode filename round-trips through Save-As intact. Same shape was latent in three siblings and fixed in the same PR (no deferred follow-ups): archive QR endpoint (archive.print_name from 3MF metadata), project ZIP export (project.name — the existing isalnum() sanitiser passes non-ASCII through), and the PDF label streamer (latent today, callers ASCII-only but the helper hardens it).
This commit is contained in:
@@ -4,6 +4,9 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
|
||||
## [0.2.4b4] - Unreleased
|
||||
|
||||
### Fixed
|
||||
- **Printer file download 500'd on non-ASCII filenames; same crash latent in three sibling endpoints** ([#1245](https://github.com/maziggy/bambuddy/issues/1245), reported by @1000Delta) — `GET /api/v1/printers/{id}/files/download?path=...` raised `UnicodeEncodeError: 'latin-1' codec can't encode characters in position …` for any path whose filename carried non-ASCII characters (Chinese, Japanese, Arabic, accented Latin), reproducible against P2S firmware on macOS but not target-specific. **Cause:** the route shoved `filename` straight into `Content-Disposition: attachment; filename="{filename}"` — Starlette/uvicorn encodes response headers as latin-1, so anything outside U+0000..U+00FF crashed at write-time. Same pattern existed in three sibling endpoints reachable with user-controlled non-ASCII input: `GET /archives/{id}/qr` (uses `archive.print_name` from 3MF metadata, often non-ASCII), `GET /projects/{id}/export` (uses `project.name` — the existing sanitiser at `projects.py:1648` uses `c.isalnum()` which **passes non-ASCII Unicode through**, so the crash propagated), and `_stream_pdf` in `labels.py` (latent — current callers pass ASCII-only template names, but the same shape would crash if a future caller passed user input). **Fix:** new helper `backend/app/utils/http.py::build_content_disposition(filename, disposition="attachment")` returns an RFC 6266-compliant header with both an ASCII-stripped legacy `filename="..."` fallback and an RFC 5987 `filename*=UTF-8''<percent-encoded>` parameter — every modern browser (Chrome / Firefox / Safari / Edge) prefers the `*=` form when present, so the original filename round-trips intact through Save-As; the ASCII fallback covers IE10-era clients. Helper wired in at all four call sites in one PR (per project rule: no deferred follow-ups). **Tests:** 20 unit tests in `test_http_utils.py` pinning ASCII-fallback rules across plain ASCII / Chinese / Japanese / Arabic / French diacritics / `.gcode.3mf` double-extension / quote-injection / backslash-injection / empty-string and `___.zip` edge cases, asserting the helper's output round-trips through latin-1 (the crash condition) for every test input. 6 new integration tests in `test_printers_api.py::TestPrintersAPI::test_download_printer_file_non_ascii_filename` parametrized over the same character classes (the original `龙泡泡石墩子_p2s_ok.gcode.3mf` case from #1245 is included) — each asserts the route returns 200 with an unmangled body, the ASCII fallback in the header matches expectations, and `unquote(filename*=)` round-trips back to the original Unicode filename. Thanks to @1000Delta for the diagnosis and the proof-of-concept patch on `printers.py` — the broader audit (three sibling endpoints, helper extraction, latin-1 round-trip assertions) was done on top of that.
|
||||
|
||||
## [0.2.4b3] - 2026-05-08
|
||||
|
||||
### Added
|
||||
|
||||
@@ -27,6 +27,7 @@ from backend.app.models.user import User
|
||||
from backend.app.schemas.archive import ArchiveResponse, ArchiveSlim, ArchiveStats, ArchiveUpdate, ReprintRequest
|
||||
from backend.app.schemas.slicer import SliceRequest
|
||||
from backend.app.services.archive import ArchiveService
|
||||
from backend.app.utils.http import build_content_disposition
|
||||
from backend.app.utils.threemf_tools import (
|
||||
extract_nozzle_mapping_from_3mf,
|
||||
extract_project_filaments_from_3mf,
|
||||
@@ -633,7 +634,7 @@ async def export_archives(
|
||||
return StreamingResponse(
|
||||
io.BytesIO(file_bytes),
|
||||
media_type=content_type,
|
||||
headers={"Content-Disposition": f'attachment; filename="{filename}"'},
|
||||
headers={"Content-Disposition": build_content_disposition(filename)},
|
||||
)
|
||||
|
||||
|
||||
@@ -672,7 +673,7 @@ async def export_stats(
|
||||
return StreamingResponse(
|
||||
io.BytesIO(file_bytes),
|
||||
media_type=content_type,
|
||||
headers={"Content-Disposition": f'attachment; filename="{filename}"'},
|
||||
headers={"Content-Disposition": build_content_disposition(filename)},
|
||||
)
|
||||
|
||||
|
||||
@@ -2343,10 +2344,11 @@ async def get_qrcode(
|
||||
pil_img.save(buffer, format="PNG")
|
||||
buffer.seek(0)
|
||||
|
||||
qr_filename = f"qr_{archive.print_name or archive_id}.png"
|
||||
return Response(
|
||||
content=buffer.getvalue(),
|
||||
media_type="image/png",
|
||||
headers={"Content-Disposition": f'inline; filename="qr_{archive.print_name or archive_id}.png"'},
|
||||
headers={"Content-Disposition": build_content_disposition(qr_filename, disposition="inline")},
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -30,6 +30,7 @@ from backend.app.models.spool import Spool
|
||||
from backend.app.models.user import User
|
||||
from backend.app.services.label_renderer import LabelData, TemplateName, render_labels
|
||||
from backend.app.services.spoolman import get_spoolman_client
|
||||
from backend.app.utils.http import build_content_disposition
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -126,7 +127,7 @@ def _stream_pdf(pdf: bytes, filename: str) -> StreamingResponse:
|
||||
io.BytesIO(pdf),
|
||||
media_type="application/pdf",
|
||||
headers={
|
||||
"Content-Disposition": f'inline; filename="{filename}"',
|
||||
"Content-Disposition": build_content_disposition(filename, disposition="inline"),
|
||||
"Content-Length": str(len(pdf)),
|
||||
# PDFs are deterministic per request; tell the browser not to cache
|
||||
# so re-printing after edits picks up the new data.
|
||||
|
||||
@@ -45,6 +45,7 @@ from backend.app.services.printer_manager import (
|
||||
supports_chamber_temp,
|
||||
supports_drying,
|
||||
)
|
||||
from backend.app.utils.http import build_content_disposition
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
router = APIRouter(prefix="/printers", tags=["printers"])
|
||||
@@ -1057,7 +1058,7 @@ async def download_printer_file(
|
||||
return Response(
|
||||
content=data,
|
||||
media_type=content_type,
|
||||
headers={"Content-Disposition": f'attachment; filename="{filename}"'},
|
||||
headers={"Content-Disposition": build_content_disposition(filename)},
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -40,6 +40,7 @@ from backend.app.schemas.project import (
|
||||
ProjectUpdate,
|
||||
TimelineEvent,
|
||||
)
|
||||
from backend.app.utils.http import build_content_disposition
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -1651,7 +1652,7 @@ async def export_project(
|
||||
return StreamingResponse(
|
||||
zip_buffer,
|
||||
media_type="application/zip",
|
||||
headers={"Content-Disposition": f'attachment; filename="{filename}"'},
|
||||
headers={"Content-Disposition": build_content_disposition(filename)},
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,17 @@
|
||||
"""HTTP response helpers."""
|
||||
|
||||
from urllib.parse import quote
|
||||
|
||||
|
||||
def build_content_disposition(filename: str, disposition: str = "attachment") -> str:
|
||||
"""Build an RFC 6266-compliant Content-Disposition header value.
|
||||
|
||||
Starlette/uvicorn encodes response headers as latin-1, so any non-ASCII
|
||||
character in a raw `filename="..."` parameter raises UnicodeEncodeError.
|
||||
The fix is RFC 5987's `filename*=UTF-8''<percent-encoded>` form alongside
|
||||
a stripped ASCII fallback in the legacy `filename="..."` parameter — every
|
||||
modern browser prefers the `*` form when present.
|
||||
"""
|
||||
ascii_fallback = filename.encode("ascii", "ignore").decode("ascii").strip(" ._-") or "download"
|
||||
ascii_fallback = ascii_fallback.replace('"', "").replace("\\", "")
|
||||
return f"{disposition}; filename=\"{ascii_fallback}\"; filename*=UTF-8''{quote(filename)}"
|
||||
@@ -4,6 +4,7 @@ Tests the full request/response cycle for /api/v1/printers/ endpoints.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from urllib.parse import unquote
|
||||
|
||||
import pytest
|
||||
from httpx import AsyncClient
|
||||
@@ -231,6 +232,53 @@ class TestPrintersAPI:
|
||||
|
||||
assert response.status_code == 404
|
||||
|
||||
# ========================================================================
|
||||
# File download endpoint — non-ASCII filename regression (#1245)
|
||||
# ========================================================================
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.parametrize(
|
||||
("filename", "ascii_fallback"),
|
||||
[
|
||||
("龙泡泡石墩子_p2s_ok.gcode.3mf", "p2s_ok.gcode.3mf"),
|
||||
("こんにちは.gcode.3mf", "gcode.3mf"),
|
||||
("résumé.gcode.3mf", "rsum.gcode.3mf"),
|
||||
("مرحبا.gcode.3mf", "gcode.3mf"),
|
||||
("文件.3mf", "3mf"),
|
||||
("hello.3mf", "hello.3mf"),
|
||||
],
|
||||
)
|
||||
async def test_download_printer_file_non_ascii_filename(
|
||||
self,
|
||||
async_client: AsyncClient,
|
||||
printer_factory,
|
||||
filename: str,
|
||||
ascii_fallback: str,
|
||||
db_session,
|
||||
):
|
||||
"""Non-ASCII filenames must not crash header encoding (issue #1245)."""
|
||||
printer = await printer_factory()
|
||||
file_bytes = b"fake 3mf content"
|
||||
|
||||
with patch(
|
||||
"backend.app.api.routes.printers.download_file_bytes_async",
|
||||
new=AsyncMock(return_value=file_bytes),
|
||||
):
|
||||
response = await async_client.get(
|
||||
f"/api/v1/printers/{printer.id}/files/download",
|
||||
params={"path": f"/cache/{filename}"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert response.content == file_bytes
|
||||
|
||||
content_disposition = response.headers["content-disposition"]
|
||||
assert f'filename="{ascii_fallback}"' in content_disposition
|
||||
assert "filename*=UTF-8''" in content_disposition
|
||||
encoded_name = content_disposition.split("filename*=UTF-8''", 1)[1]
|
||||
assert unquote(encoded_name) == filename
|
||||
|
||||
# ========================================================================
|
||||
# Status endpoint
|
||||
# ========================================================================
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
"""Unit tests for backend.app.utils.http."""
|
||||
|
||||
from urllib.parse import unquote
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.app.utils.http import build_content_disposition
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("filename", "expected_ascii_fallback"),
|
||||
[
|
||||
("hello.gcode.3mf", "hello.gcode.3mf"),
|
||||
("龙泡泡石墩子_p2s_ok.gcode.3mf", "p2s_ok.gcode.3mf"),
|
||||
("こんにちは.gcode.3mf", "gcode.3mf"),
|
||||
("résumé.gcode.3mf", "rsum.gcode.3mf"),
|
||||
("مرحبا.gcode.3mf", "gcode.3mf"),
|
||||
("文件.3mf", "3mf"),
|
||||
("模型.gcode.3mf", "gcode.3mf"),
|
||||
("project_2026-05-08.zip", "project_2026-05-08.zip"),
|
||||
("___.zip", "zip"),
|
||||
("", "download"),
|
||||
],
|
||||
)
|
||||
def test_ascii_fallback_strips_non_ascii(filename: str, expected_ascii_fallback: str) -> None:
|
||||
header = build_content_disposition(filename)
|
||||
assert f'filename="{expected_ascii_fallback}"' in header
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"filename",
|
||||
[
|
||||
"龙泡泡石墩子_p2s_ok.gcode.3mf",
|
||||
"こんにちは.gcode.3mf",
|
||||
"résumé.gcode.3mf",
|
||||
"مرحبا.gcode.3mf",
|
||||
"文件.3mf",
|
||||
"hello world (final).pdf",
|
||||
"你好/世界.pdf",
|
||||
],
|
||||
)
|
||||
def test_filename_star_round_trips_to_original(filename: str) -> None:
|
||||
header = build_content_disposition(filename)
|
||||
assert "filename*=UTF-8''" in header
|
||||
encoded = header.split("filename*=UTF-8''", 1)[1]
|
||||
assert unquote(encoded) == filename
|
||||
|
||||
|
||||
def test_header_is_latin1_encodable() -> None:
|
||||
"""Starlette/uvicorn encodes response headers as latin-1 — the helper's
|
||||
output MUST round-trip through latin-1 without raising."""
|
||||
for filename in [
|
||||
"龙泡泡石墩子_p2s_ok.gcode.3mf",
|
||||
"こんにちは.gcode.3mf",
|
||||
"résumé.gcode.3mf",
|
||||
"مرحبا.gcode.3mf",
|
||||
'"quoted"name.zip',
|
||||
"back\\slash.zip",
|
||||
]:
|
||||
header = build_content_disposition(filename)
|
||||
header.encode("latin-1")
|
||||
|
||||
|
||||
def test_disposition_param_is_respected() -> None:
|
||||
assert build_content_disposition("foo.pdf", disposition="inline").startswith("inline; ")
|
||||
assert build_content_disposition("foo.pdf").startswith("attachment; ")
|
||||
|
||||
|
||||
def test_quotes_and_backslashes_stripped_from_ascii_fallback() -> None:
|
||||
header = build_content_disposition('a"b\\c.pdf')
|
||||
assert 'filename="abc.pdf"' in header
|
||||
Reference in New Issue
Block a user