Skip to content

feat: add --url-prefix flag for reverse proxy subpath support#309

Merged
dubadub merged 2 commits intomainfrom
feat/url-prefix
Apr 6, 2026
Merged

feat: add --url-prefix flag for reverse proxy subpath support#309
dubadub merged 2 commits intomainfrom
feat/url-prefix

Conversation

@dubadub
Copy link
Copy Markdown
Member

@dubadub dubadub commented Apr 5, 2026

Summary

  • Adds --url-prefix CLI flag to cook server for running behind a reverse proxy at a subpath (e.g., cook server --url-prefix "/cook")
  • Uses Axum's Router::nest() for route mounting and NormalizePath middleware for trailing slash handling
  • Updates all templates, fetch calls, navigation links, static asset URLs, and keyboard shortcuts to respect the prefix via template variable and window.__PREFIX__ JS global

Test plan

  • cargo fmt — passes
  • cargo clippy — passes
  • cargo test — passes
  • Manual: cook server --url-prefix "/cook" ./seed serves UI at /cook/
  • Manual: /cook/ and /cook both return 200
  • Manual: / returns 404 when prefix is set
  • Manual: All navigation links, static assets, API calls, and search work under prefix
  • Manual: Without --url-prefix, server works as before at root /
  • Manual: WebSocket LSP connection works under prefix in edit mode

Closes #300

Allow the server to run behind a reverse proxy at a subpath (e.g.,
--url-prefix "/cook" serves all routes under /cook/). Uses Axum's
Router::nest() for clean route mounting and NormalizePath middleware
to handle trailing slashes. All templates, fetch calls, navigation
links, and static asset URLs respect the configured prefix.

Closes #300
@claude
Copy link
Copy Markdown

claude bot commented Apr 5, 2026

PR Review: feat: add --url-prefix flag for reverse proxy subpath support

Overall this is a well-structured feature with thorough coverage across templates, JS, and the Axum routing layer. The approach of using Router::nest() + NormalizePath is idiomatic. A few observations:


Correctness

window.__PREFIX__ injection uses HTML escaping in a JS context (base.html):

<script>window.__PREFIX__ = "{{ prefix }}";</script>

Askama HTML-escapes values by default (<&lt;, &&amp;), but inside <script> blocks HTML entities are not decoded — they appear as literal strings in JS. So a prefix like /cook&test would set window.__PREFIX__ to the string "/cook&amp;test" instead of "/cook&test".

Fix: use the json filter (already used elsewhere in the codebase for recipePath in edit.html):

<script>window.__PREFIX__ = {{ prefix|json|safe }};</script>

Behavioral Change

NormalizePath is now applied unconditionally (mod.rs):

let app = tower_http::normalize_path::NormalizePath::trim_trailing_slash(app);

This silently changes behavior when running without --url-prefix — trailing slashes are now stripped from all routes. This is probably fine (and arguably desirable), but worth calling out in the PR description/changelog since it affects existing deployments.


Robustness

Validation could be stricter in build_state(). The current check only rejects strings that don't start with /. Characters like ", ', spaces, or newlines won't be caught but can cause issues in template HTML attributes and the cookie path. Consider rejecting anything outside valid URL path segment characters.


Minor Nits

  • CSS selector with prefix in base.html (the .dark a[href="{{ prefix }}/"] rule): same HTML-escaping-in-CSS-context caveat as the JS case, though rare for typical URL prefixes.
  • Cookie path in preferences.html: path={{ prefix }}/ is correct, but note this means language preference cookies won't persist if a user ever accesses the non-prefixed root. Likely intentional.
  • The let prefix = state.url_prefix.clone(); capture before moving state into menu_page_handler in ui.rs is intentional and correct — just noting it for reviewers.

Overall

The implementation is comprehensive and well thought out. The |json|safe filter fix for the __PREFIX__ global is the most important change to make before merging. The rest are minor improvements. The manual test plan looks solid.

@pinage404
Copy link
Copy Markdown
Contributor

This will also help with this #186

Address PR review feedback:
- Use |json|safe filter for window.__PREFIX__ to avoid HTML entity
  encoding issues in JS context
- Validate URL prefix contains only safe URL path characters
  (alphanumeric, /, -, _, .)
@claude
Copy link
Copy Markdown

claude bot commented Apr 6, 2026

PR Review: feat: add --url-prefix flag for reverse proxy subpath support

Overall this is a well-structured, useful feature. The implementation is thorough — all templates, JS fetch calls, and keyboard shortcuts have been updated consistently.

Correctness

Input validation is solid. The allowlist (alphanumeric, slash, dash, underscore, dot) covers legitimate subpaths and excludes characters that could cause issues in CSS selectors, redirects, or template injection.

|json|safe for window.__PREFIX__ — exactly right. Injecting a server-side value directly into a script block without the json filter would HTML-entity-encode quotes, breaking JS syntax. The second commit addresses this correctly.

Redirect fix for args.open — moving build_state before the open check is correct since args is moved into build_state. Extracting let open = args.open; first handles this cleanly.


Potential Issues

Edge case: --url-prefix "/" produces a confusing error.

After trim_end_matches('/'), the value / becomes an empty string, which then fails starts_with('/') with the message: "URL prefix must start with '/', got: ". The error shows an empty string, not the original input /. Validating before stripping would give a clearer message.

NormalizePath is applied globally, even without a prefix set.

let app = tower_http::normalize_path::NormalizePath::trim_trailing_slash(app);

This wraps every request unconditionally. For users not using --url-prefix, this is a silent behavior change: trailing slashes will now be stripped from all routes. In practice this is likely benign, but worth calling out in release notes since it affects the default (no-prefix) behavior.

Cookie path scoping in preferences.html.

The cookie is set with path=<prefix>/. When prefix is empty this correctly becomes path=/. When prefix is /cook it becomes path=/cook/, which correctly scopes the cookie. The logic is correct, but worth a manual test to confirm language preference persists across page reloads when a prefix is set.


Minor Notes

  • prefix: String cloned per template is perfectly fine for a value set once at startup.
  • The get_image_path refactor (flattening the nested if/else) is equivalent logic and cleaner. Good opportunistic cleanup.
  • The manual test checklist is comprehensive. The WebSocket LSP connection under a prefix is particularly important to verify end-to-end.

Summary

Category Assessment
Correctness Good — consistent and complete coverage of all URL construction sites
Security Good — strict input validation, json+safe filter in JS context
Behavior change risk Low — NormalizePath side effect on non-prefixed deploys worth noting
Code quality Clean — validation is readable, template changes are mechanical and systematic

The single-slash edge case and the unconditional NormalizePath wrapping are the two things worth addressing before merge. Everything else looks solid.

@dubadub dubadub merged commit bdf4d32 into main Apr 6, 2026
6 checks passed
@dubadub dubadub deleted the feat/url-prefix branch April 6, 2026 19:23
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.

[feature ?] base url parameter

2 participants