Skip to content

fix(backend): serve PDF/Markdown manuals inline so the viewer can render them#3714

Merged
gantoine merged 2 commits into
masterfrom
posthog-code/fix-manual-inline-viewer
Jul 10, 2026
Merged

fix(backend): serve PDF/Markdown manuals inline so the viewer can render them#3714
gantoine merged 2 commits into
masterfrom
posthog-code/fix-manual-inline-viewer

Conversation

@gantoine

@gantoine gantoine commented Jul 9, 2026

Copy link
Copy Markdown
Member

Description
Explain the changes or enhancements you are proposing with this pull request.

Uploaded manuals (a RomFile with category="manual") are served from GET /api/roms/{id}/files/content/{file_name}. That endpoint only treated soundtracks and image/video files as inline media, so manuals fell through to application/octet-stream + Content-Disposition: attachment. The same URL is used for both viewing and downloading, so downloads and the nav chrome kept working while the in-page PDF/Markdown viewer got an octet-stream/attachment response and rendered a blank area.

This only affected manuals uploaded as extra files. Scraped/primary manuals (rom.has_manual + rom.path_manual) load from the static-asset path and were unaffected, which is why the bug reproduced for some libraries but not others.

Changes:

  • Add .pdf and .md to is_allowed_media_file (via a new ALLOWED_DOCUMENT_EXTENSIONS / is_allowed_document_file), with explicit MIME overrides (application/pdf, text/markdown) so the content type is trustworthy across platforms. Manuals are now served inline.
  • Set X-Content-Type-Options: nosniff on inline responses (both the dev FileResponse and the nginx FileRedirectResponse). SVG is deliberately kept out of the inline allowlist because it can carry script, so serving Markdown inline warrants the same care: nosniff prevents the browser from sniffing a .md manual into HTML.
  • Add endpoint tests covering the previously-untested manual content path.

Fixes #3712

AI assistance disclosure
Per CONTRIBUTING.md.

This change was written with AI assistance (PostHog Code / Claude): investigation, code changes, and tests were AI-generated and human-reviewed.

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

Screenshots (if applicable)


Created with PostHog Code

…der them

Uploaded manual files (RomFile with category="manual") were served from the
content endpoint as application/octet-stream + attachment, so the in-page
manual viewer showed a blank area while nav buttons and download still worked.

Include .pdf and .md in is_allowed_media_file (with explicit MIME overrides)
so they are served inline, and set X-Content-Type-Options: nosniff on inline
responses so a Markdown manual can't be sniffed into HTML.

Fixes #3712

Generated-By: PostHog Code
Task-Id: 73be371c-908a-4f9f-9748-7ddba2df71e2
Copilot AI review requested due to automatic review settings July 9, 2026 23:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes rendering of uploaded ROM manuals in the in-page manual viewer by serving PDF/Markdown files inline (with trusted MIME types) from the existing ROM file content endpoint, rather than forcing them to download as attachments.

Changes:

  • Extend the inline-media allowlist to include .pdf and .md, with explicit MIME overrides (application/pdf, text/markdown).
  • Add X-Content-Type-Options: nosniff for inline responses (both dev FileResponse and nginx X-Accel-Redirect responses).
  • Add endpoint tests asserting inline disposition, correct Content-Type, and nosniff for PDF/Markdown manuals.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
backend/utils/media_types.py Adds document extensions and MIME overrides so PDFs/Markdown can be served inline with consistent Content-Type.
backend/endpoints/roms/files.py Applies nosniff to inline responses and forwards headers through both FileResponse and FileRedirectResponse.
backend/tests/endpoints/roms/test_files.py Adds regression tests for serving PDF/Markdown manual files inline with expected headers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread backend/utils/media_types.py
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR lets uploaded manuals render inline from the ROM file content endpoint. The main changes are:

  • Added PDF and Markdown to inline document handling.
  • Added explicit MIME overrides for .pdf and .md.
  • Added X-Content-Type-Options: nosniff to inline responses.
  • Added endpoint tests for uploaded PDF and Markdown manuals.

Confidence Score: 5/5

The changed flow looks mergeable after a small cleanup to the document inline scope.

  • PDF and Markdown manuals now get the headers the viewer needs.
  • The response header wiring is compatible with both serving paths.
  • Non-manual .pdf and .md files can now be served inline because the generic media helper changed.

backend/utils/media_types.py

Important Files Changed

Filename Overview
backend/utils/media_types.py Adds document extensions and MIME overrides to the generic inline media helper.
backend/endpoints/roms/files.py Adds nosniff headers to inline file responses on both direct and nginx redirect paths.
backend/tests/endpoints/roms/test_files.py Adds tests for uploaded PDF and Markdown manual content responses.

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(backend): serve PDF/Markdown manuals..." | Re-trigger Greptile

Comment thread backend/utils/media_types.py
greptile-apps[bot]
greptile-apps Bot previously approved these changes Jul 9, 2026
Serving any .pdf/.md file inline by extension also flipped game/extra files
from downloads to inline. Gate the document check on RomFileCategory.MANUAL so
only manuals render inline; other files keep the attachment behavior.

Generated-By: PostHog Code
Task-Id: 73be371c-908a-4f9f-9748-7ddba2df71e2
@greptile-apps greptile-apps Bot dismissed their stale review July 9, 2026 23:57

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

@gantoine gantoine merged commit d200fa4 into master Jul 10, 2026
10 checks passed
@gantoine gantoine deleted the posthog-code/fix-manual-inline-viewer branch July 10, 2026 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 5.0.0 Manuals not displaying

2 participants