diff --git a/AGENTS.md b/AGENTS.md index 9ddf67c..d9e0909 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -383,7 +383,18 @@ Consequences that are easy to get wrong: - **Cookie `Path` is scoped to the prefix** (`Path=/forgejo`), so the session token is not sent to sibling apps on the same host. At the root it is `/`, the default-path. - **Static assets stay app-absolute** (`nest_service("/static")` in `routes::app`). This is the one place the base path is *not* applied, and getting it wrong is subtle: the asset 404s, `x-data="app()"` never initialises, and every Alpine directive — including the File menu dropdowns — silently stops working. There is a test asserting `GET /static/app.js` returns 200 while a base path is set. `app.js` is inlined via `include_str!` and has no URL to rewrite; only `alpine.min.js` and `style.css` are externally loaded. - **Curl will 404 on prefixed paths.** Hitting `http://host:3000/forgejo/login` directly bypasses Apache, so nothing strips the prefix. Test through the proxy, or curl the app-absolute path. -- **Never hardcode a path in JS.** `static/app.js` matches the new-note form with `form[action$="/notes"]` rather than an exact attribute, so `Ctrl+N` keeps working under any prefix. +- **Never hardcode a path in JS.** `static/app.js` matches the new-note form with `form[action$="/notes"]` rather than an exact attribute, so `Ctrl+N` keeps working under any prefix. The one `fetch()` it makes is different, because the rule above does not cover it: + + ```javascript + const base = window.__BASE_PATH__ || ''; + fetch(base + '/preview', { ... }); + ``` + + `app.js` is inlined into the page, so it cannot call the server-side `base.url()` helper that every form action and asset `src` uses. A literal `fetch('/preview')` resolves against the *origin root*, which behind a proxy mounted at `/forgejo/` never reaches the app. The failure is nasty because the proxy returns its own 404 body, which `x-html` then swaps straight into the preview pane — the user sees "404 Not Found" in the middle of their document while typing, with nothing in the server logs to explain it. `render_app` therefore injects `window.__BASE_PATH__` next to `__INITIAL_PREVIEW__`, escaped by the same `escape_js` helper and emitted as a backtick literal like its neighbour. + + Pinned by `the_base_path_is_handed_to_client_side_js` and `the_preview_fetch_uses_the_injected_base_path` in `tests/auth_gating.rs`. The second matters most: `app.js` is inlined verbatim, so a regression there is invisible to every other test in the suite. + +- **A redirect target is not a forwarded path.** A `303` to `{base}/` is correct for the browser and 404s if you curl it directly, because nothing strips the prefix. This also bites reverse-proxy test harnesses: a proxy built on `urllib`/`requests` will silently follow the redirect, re-resolve it against the upstream root, and return a 404 that looks like an app bug. Disable redirect following when testing through a proxy. --- diff --git a/README.md b/README.md index 6bd2734..6f99b64 100644 --- a/README.md +++ b/README.md @@ -70,8 +70,8 @@ cargo run -- --base-path /forgejo ``` ```apache -ProxyPass /forgejo/ http://127.0.0.1:3000/ timeout=10 -ProxyPassReverse /forgejo/ http://127.0.0.1:3000/ +ProxyPass /el-notes/ http://127.0.0.1:3000/ timeout=10 +ProxyPassReverse /el-notes/ http://127.0.0.1:3000/ ``` The proxy strips the prefix, so the app still sees ordinary paths and adds the prefix back when generating links, form actions, and redirects. The session cookie's `Path` is scoped to the prefix, so the token is not sent to sibling apps on the same host. diff --git a/src/pages.rs b/src/pages.rs index d813c6c..4a4d7c4 100644 --- a/src/pages.rs +++ b/src/pages.rs @@ -21,6 +21,17 @@ impl<'a> Base<'a> { pub fn url(&self, path: &str) -> String { format!("{}{}", self.0, path) } + + /// The raw prefix, for handing to client-side JS. + /// + /// `url()` covers every URL in the rendered markup. This exists for the + /// one `fetch()` in `app.js`, which has to build the URL client-side and + /// would otherwise resolve `/preview` against the origin root — behind a + /// reverse proxy mounted at a subdirectory that misses the app entirely + /// and returns the proxy's own 404. + pub fn prefix(&self) -> &str { + self.0 + } } pub fn render_app( @@ -72,6 +83,7 @@ pub fn render_app( (render_status_bar(notes.len(), theme_name)) script { "window.__INITIAL_PREVIEW__ = `" (PreEscaped(escape_js(&rendered_preview))) "`;" + "window.__BASE_PATH__ = `" (PreEscaped(escape_js(base.prefix()))) "`;" (PreEscaped(include_str!("../static/app.js"))) } } diff --git a/static/app.js b/static/app.js index 594fe6c..7f55f91 100644 --- a/static/app.js +++ b/static/app.js @@ -53,7 +53,16 @@ function app() { const formData = new URLSearchParams(); formData.append('content', content); - fetch('/preview', { + // The base path has to be prepended here. `app.js` is inlined into + // the page, so it cannot call the server-side `base.url()` helper + // that every form action uses — and an origin-absolute `/preview` + // resolves against the site root, which behind a reverse proxy + // mounted at a subdirectory never reaches the app. The response body + // would then be the proxy's own 404, swapped straight into the + // preview pane. + const base = window.__BASE_PATH__ || ''; + + fetch(base + '/preview', { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body: formData.toString() diff --git a/tests/auth_gating.rs b/tests/auth_gating.rs index 0b26df2..fd7d7b7 100644 --- a/tests/auth_gating.rs +++ b/tests/auth_gating.rs @@ -720,6 +720,60 @@ async fn every_screen_declares_a_viewport() { } } +/// `app.js` is inlined into the page, so it cannot call the server-side +/// `base.url()` helper. Its one `fetch()` has to build the URL itself, which +/// is what `window.__BASE_PATH__` is for. +/// +/// Without it, `fetch('/preview')` resolves against the origin root. Behind +/// a reverse proxy mounted at a subdirectory that never reaches the app, and +/// the proxy's own 404 body gets swapped into the preview pane. The symptom +/// is a preview pane full of "404 Not Found" while typing. +#[tokio::test] +async fn the_base_path_is_handed_to_client_side_js() { + // With a base path set, the browser must be told about it. + let f = Fixture::with_base_path("base-js", "/forgejo") + .login_over_http() + .await; + let (status, body) = f.get_authed("/").await; + assert_eq!(status, StatusCode::OK); + // Backtick template literal, matching the `__INITIAL_PREVIEW__` line + // above it and escaped by the same `escape_js` helper. + assert!( + body.contains("window.__BASE_PATH__ = `/forgejo`;"), + "client JS should be handed the base path" + ); + + // Without one, the value must be empty rather than the string "undefined", + // so the fetch URL stays byte-identical to the pre-fix behaviour. + let plain = Fixture::fresh("base-js-none").login_over_http().await; + let (status, body) = plain.get_authed("/").await; + assert_eq!(status, StatusCode::OK); + assert!( + body.contains("window.__BASE_PATH__ = ``;"), + "with no base path the client value should be empty" + ); +} + +/// The `fetch()` in `app.js` must actually use the injected value. This is +/// the assertion that would have caught the original bug: `app.js` is +/// inlined verbatim, so a regression there is invisible to every other test. +#[tokio::test] +async fn the_preview_fetch_uses_the_injected_base_path() { + let f = Fixture::with_base_path("fetch-base", "/forgejo") + .login_over_http() + .await; + let (status, body) = f.get_authed("/").await; + assert_eq!(status, StatusCode::OK); + assert!( + body.contains("fetch(base + '/preview'"), + "app.js should build the preview URL from __BASE_PATH__" + ); + assert!( + !body.contains("fetch('/preview'"), + "app.js must not hardcode an origin-absolute /preview" + ); +} + /// The about page advertises a version, and it must be the one in /// `Cargo.toml`. ///