From 3f58fc74b4d0204241100e2acf23d202f9e5b5b5 Mon Sep 17 00:00:00 2001 From: maziggy Date: Fri, 8 May 2026 14:06:40 +0200 Subject: [PATCH] fix(http): RFC 6266-encode Content-Disposition so non-ASCII filenames don't crash response (issue #1245) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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'' 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). --- CHANGELOG.md | 3 + backend/app/api/routes/archives.py | 8 ++- backend/app/api/routes/labels.py | 3 +- backend/app/api/routes/printers.py | 3 +- backend/app/api/routes/projects.py | 3 +- backend/app/utils/http.py | 17 +++++ .../tests/integration/test_printers_api.py | 48 +++++++++++++ backend/tests/unit/test_http_utils.py | 71 +++++++++++++++++++ 8 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 backend/app/utils/http.py create mode 100644 backend/tests/unit/test_http_utils.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6acf9fab2..fa45b932b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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''` 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 diff --git a/backend/app/api/routes/archives.py b/backend/app/api/routes/archives.py index 8ff97ff22..de2e4102b 100644 --- a/backend/app/api/routes/archives.py +++ b/backend/app/api/routes/archives.py @@ -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")}, ) diff --git a/backend/app/api/routes/labels.py b/backend/app/api/routes/labels.py index 9e30bc51f..081b2dd65 100644 --- a/backend/app/api/routes/labels.py +++ b/backend/app/api/routes/labels.py @@ -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. diff --git a/backend/app/api/routes/printers.py b/backend/app/api/routes/printers.py index b700d3a4b..2d4282745 100644 --- a/backend/app/api/routes/printers.py +++ b/backend/app/api/routes/printers.py @@ -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)}, ) diff --git a/backend/app/api/routes/projects.py b/backend/app/api/routes/projects.py index 67230956d..07d66c53a 100644 --- a/backend/app/api/routes/projects.py +++ b/backend/app/api/routes/projects.py @@ -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)}, ) diff --git a/backend/app/utils/http.py b/backend/app/utils/http.py new file mode 100644 index 000000000..8aef07222 --- /dev/null +++ b/backend/app/utils/http.py @@ -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''` 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)}" diff --git a/backend/tests/integration/test_printers_api.py b/backend/tests/integration/test_printers_api.py index bd11b292e..b64785262 100644 --- a/backend/tests/integration/test_printers_api.py +++ b/backend/tests/integration/test_printers_api.py @@ -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 # ======================================================================== diff --git a/backend/tests/unit/test_http_utils.py b/backend/tests/unit/test_http_utils.py new file mode 100644 index 000000000..3692a843a --- /dev/null +++ b/backend/tests/unit/test_http_utils.py @@ -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