Skip to content

fix(templates): respect reverse-proxy path prefix in hardcoded JS/URLs - #16

Closed
jeromelefeuvre wants to merge 1 commit into
ndandan:mainfrom
jeromelefeuvre:fix/hardcoded-prefix-paths
Closed

fix(templates): respect reverse-proxy path prefix in hardcoded JS/URLs#16
jeromelefeuvre wants to merge 1 commit into
ndandan:mainfrom
jeromelefeuvre:fix/hardcoded-prefix-paths

Conversation

@jeromelefeuvre

@jeromelefeuvre jeromelefeuvre commented Jul 24, 2026

Copy link
Copy Markdown

Summary

When Prismarr is served behind a reverse proxy that strips a path prefix (e.g. Traefik routing https://host/prismarr/... to the container root, with X-Forwarded-Prefix: /prismarr forwarded), server-generated URLs are already prefix-safe via instance_path() / path() (which go through the Symfony router, itself driven by Request::getBasePath() — already trusted via trusted_headers: [..., x-forwarded-prefix] in config/packages/framework.yaml).

However, ~276 places across 67 Twig templates build absolute paths as raw string literals inside inline <script> blocks or href/action attributes (fetch('/medias/' + slug + ...), href="/jellyseerr/...", etc.). These literals ignore the prefix entirely, since X-Forwarded-Prefix only affects server-side URL generation, not raw JS strings.

Fix

  • Standard case (raw text inside a <script> block or HTML attribute): prefix the literal with {{ app.request.getBasePath() }}.
  • Edge case (~9 occurrences already inside an open Twig expression, e.g. {% include ... with { save_url_edit: ... } %} or a ternary): use the Twig concat operator app.request.getBasePath() ~ '...' instead, since {{ }} doesn't interpolate inside an already-parsed Twig expression.
  • One occurrence (/api/health/services) was switched to the named route via {{ path('api_health_services') }} instead, which is cleaner and needed no raw-path handling.

Request::getBasePath() already reflects X-Forwarded-Prefix (Symfony 5.3+), so this resolves to /prismarr behind the proxy and '' on a root deployment — no extra config needed.

Affected roots: /medias, /decouverte, /jellyseerr, /prowlarr, /setup.

Test plan

  • make lint-twig (php bin/console lint:twig templates) — all 149 Twig files pass syntax validation
  • Swept the repo for any remaining hardcoded absolute paths on the 5 affected roots — none left
  • Manually verified each of the 9 "already inside a Twig expression" cases renders correctly with ~ instead of {{ }}

@jeromelefeuvre
jeromelefeuvre force-pushed the fix/hardcoded-prefix-paths branch from 290d73c to 6a1fd41 Compare July 24, 2026 04:18
@jeromelefeuvre

Copy link
Copy Markdown
Author

Salut! ton projet avance plus vite que l'original, je t'envois quelques PRs ;)

@ndandan

ndandan commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Thanks for this — I read the whole diff, and the change itself is clean: 421 added lines, 420 of which are purely the {{ app.request.getBasePath() }} prefix, one switched to a named route. No behaviour change on a root deployment, since getBasePath() resolves to '' there. The ~ handling for the ~9 already-inside-a-Twig-expression cases is right, and x-forwarded-prefix is indeed already in trusted_headers, so the premise holds.

I'm going to hold this on the fork rather than merge it, for reasons that aren't about the patch quality:

  1. It's open upstream as fix(templates): respect reverse-proxy path prefix in hardcoded JS/URLs Shoshuo/Prismarr#80. That's the right home for it — it's a general deployment fix, not a fork-specific one. If it lands upstream I get it on the next sync.
  2. 67 templates is the worst possible conflict surface for a fork. This fork's standing policy is staying mergeable with upstream. Merging a 67-file sweep here and then merging the same sweep down from upstream later means resolving it twice.
  3. It's still a draft, so I'm assuming you're not asking for a merge decision yet anyway.

One substantive note for whichever repo takes it: this introduces a second prefix convention. There's already a named-route pattern in the tree —

var BASE = '{{ path("app_qbittorrent_index") }}';   {# templates/qbittorrent/index.html.twig:849 #}
var BASE = '{{ path("app_deluge_index") }}';        {# templates/deluge/index.html.twig:793 #}

— and you used exactly that approach for the one api_health_services case. Named routes are the better pattern: they survive route renames, and they don't rely on HTML autoescaping being harmless inside a <script> block. A var BASE = ... per template plus BASE + '/medias/...' would also shrink the diff substantially. Not a blocker, just the direction I'd push if the maintainer asks.

Happy to revisit here if Shoshuo#80 stalls upstream.

@jeromelefeuvre
jeromelefeuvre marked this pull request as ready for review July 26, 2026 22:28
@jeromelefeuvre
jeromelefeuvre force-pushed the fix/hardcoded-prefix-paths branch from 6a1fd41 to db46fcc Compare July 27, 2026 14:12
@jeromelefeuvre
jeromelefeuvre marked this pull request as draft July 27, 2026 14:26
@jeromelefeuvre
jeromelefeuvre force-pushed the fix/hardcoded-prefix-paths branch from db46fcc to d71b393 Compare July 27, 2026 14:32
Inline JS and href/action attributes across /medias, /decouverte,
/jellyseerr, /prowlarr and /setup templates hardcoded absolute paths
(fetch('/medias/...'), href="/jellyseerr/...", etc.), which ignore the
X-Forwarded-Prefix header set by a reverse proxy stripping a path
prefix (e.g. Traefik serving the app under /prismarr/). Symfony's
Request::getBasePath() already reflects that header, so prefix each
hardcoded literal with app.request.getBasePath() (or, inside an
already-open Twig expression, with the ~ concat operator) instead of
relying on instance_path()/path() helpers that aren't used everywhere.

Each affected template now computes the prefix once into a local
`var BASE` (matching the existing convention in qbittorrent/index and
deluge/index) instead of repeating the Twig call inline at every call
site, shrinking the diff and keeping a single point of change per
template.

The initial sweep missed the app-wide quick-look modal (_quicklook.html.twig,
shared by the dashboard, calendar, Explorer and top-bar search), the
top-bar search's own quicklook URLs in base.html.twig, the Plex info
modal (dashboard/_plex_info_modal.html.twig) and the entire Tautulli
dashboard page (tautulli/index.html.twig) — none of those were under
/medias, /decouverte, /jellyseerr, /prowlarr or /setup, so they kept
hardcoded absolute paths that broke behind a stripped path prefix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jeromelefeuvre
jeromelefeuvre force-pushed the fix/hardcoded-prefix-paths branch from d71b393 to ce1adc8 Compare July 28, 2026 03:51
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.

2 participants