Fix home and post-delete redirects behind a proxy URL prefix

[AkprasadoP]

What

Preserve the public URL prefix when navigating home from a pad and after a pad is deleted.

Why

When Etherpad is served behind a reverse proxy under a prefix such as /etherpad, the Home command resolved ../.. from /etherpad/p/<padId> and redirected to the domain root. Post-delete redirects separately hardcoded /, causing the same problem.

Changes

• Resolve the home URL relative to the current pad URL.
• Use the same resolver for Home and all post-delete redirects.
• Add regression coverage for root, single-prefix, and nested-prefix deployments.

Fixes #8111

Tests

• vitest run tests/backend-new/specs/getHomeUrl.test.ts
• Playwright editbar.spec.ts

Note: ESLint could not be run locally because of a pre-existing ESLint/package compatibility mismatch.

[qodo-free-for-open-source-projects]

Code Review by Qodo

New Review Started

This review has been superseded by a new analysis

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Preserve proxy URL prefixes on Home and post-delete redirects

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add a shared URL resolver to compute Etherpad home from the current pad URL.
• Use the resolver for the Home command and all post-delete redirect paths.
• Add regression tests for root, prefixed, and nested-prefix deployments.

Diagram

graph TD
  A["Browser UI"] --> B["pad_editbar.ts"] --> D["getHomeUrl.ts"] --> E["window.location.href"]
  A["Browser UI"] --> C["pad_editor.ts"] --> D["getHomeUrl.ts"]
  F["Tests"] --> G["getHomeUrl.test.ts"] --> D["getHomeUrl.ts"]
  F["Tests"] --> H["editbar.spec.ts"] --> B["pad_editbar.ts"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Server-provided base/prefix (clientVars)

• ➕ Decouples redirect behavior from URL shape assumptions (e.g., if /p/ ever changes).
• ➕ Single canonical source of truth for the public base URL across the app.
• ➖ Requires plumbing config into the client and ensuring it reflects externally visible proxy paths.
• ➖ More moving parts than a simple URL-relative computation.

2. Pathname parsing instead of URL-relative resolution

• ➕ Avoids reliance on new URL('..', ...) semantics and can be explicit about expected segments.
• ➖ More error-prone across nested prefixes, query strings, trailing slashes, and future route changes.
• ➖ Duplicates URL parsing logic that the platform already provides.

Recommendation: The PR’s approach (derive home via new URL('..', currentHref)) is the simplest reliable fix for reverse-proxy prefixes and keeps logic centralized in one helper. If future routes diverge from {prefix}/p/{padId}, consider augmenting with a server-provided public base URL to avoid coupling to path structure.

Files changed (5) +43 / -8

Bug fix (3) +21 / -7

getHomeUrl.tsAdd shared helper to resolve home URL from a pad URL +12/-0

Add shared helper to resolve home URL from a pad URL

• Introduces 'getHomeUrl()' that computes the Etherpad home URL by resolving one '..' segment from a pad URL ('{prefix}/p/{padId}' → '{prefix}/'). This preserves reverse-proxy URL prefixes and works for root and nested-prefix deployments.

src/static/js/getHomeUrl.ts

pad_editbar.tsUse getHomeUrl() for the Home command redirect +4/-3

Use getHomeUrl() for the Home command redirect

• Replaces the previous '../..'-based redirect with a call to 'getHomeUrl()'. Ensures the Home action navigates to the correct prefixed home path when Etherpad is served under a proxy prefix.

src/static/js/pad_editbar.ts

pad_editor.tsUse getHomeUrl() for all post-delete redirects +5/-4

Use getHomeUrl() for all post-delete redirects

• Replaces hardcoded '/' redirects in pad deletion/disconnect handling with 'getHomeUrl()'. Aligns deletion navigation behavior with Home and preserves proxy prefixes in both normal and fallback timeouts.

src/static/js/pad_editor.ts

Tests (2) +22 / -1

getHomeUrl.test.tsAdd unit tests for home URL resolution across prefixes +20/-0

Add unit tests for home URL resolution across prefixes

• Adds Vitest coverage for root deployment, single proxy prefix, and deep nested proxy prefixes. Validates 'getHomeUrl()' returns the correct home URL for each input href.

src/tests/backend-new/specs/getHomeUrl.test.ts

editbar.spec.tsTighten editbar Home navigation assertion +2/-1

Tighten editbar Home navigation assertion

• Extends the Playwright editbar test to assert the final pathname is exactly '/' after navigating Home. Provides regression confidence that Home navigation lands on the expected home path.

src/tests/frontend-new/specs/editbar.spec.ts

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Browser-only getHomeUrl default ✓ Resolved 🐞 Bug ☼ Reliability

Description

getHomeUrl() defaults fromHref to window.location.href, so calling getHomeUrl() without an
argument in a non-browser runtime will throw ReferenceError: window is not defined. This is a
latent reliability/maintainability footgun now that the helper is imported from backend tests and
could be reused elsewhere; it is safe as long as non-browser callers always pass fromHref.

Code

src/static/js/getHomeUrl.ts[R11-12]

+export const getHomeUrl = (fromHref: string = window.location.href): string =>
+  new URL('..', fromHref).href;

Relevance

●●● Strong

Team often accepts defensive guards to prevent runtime crashes; safe to avoid unguarded browser
globals in shared helpers.

PR-#7688
PR-#7667
PR-#7479

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper’s default parameter directly references window, which is not defined outside browsers;
the new backend test imports this helper from a Node-side test directory, so the function is now
more likely to be reused in non-browser contexts where the no-argument form would crash.

src/static/js/getHomeUrl.ts[11-12]
src/tests/backend-new/specs/getHomeUrl.test.ts[3-5]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getHomeUrl()` uses `window.location.href` as a default parameter value. That makes `getHomeUrl()` crash if it is ever invoked without an argument in a non-browser environment (Node, server-side rendering, some test environments).

### Issue Context
This helper is now imported by Node-run backend tests, which increases the chance of it being reused outside the browser in the future.

### Fix Focus Areas
- src/static/js/getHomeUrl.ts[11-12]

### Suggested fix
Make the default safe by guarding access to `window`, for example:

- Change the signature to require `fromHref` (no default) and keep browser call sites passing `window.location.href`, **or**
- Keep an optional parameter but implement a guarded default:
 - `const href = fromHref ?? (typeof window !== 'undefined' ? window.location.href : '');`
 - If `href` is empty, throw a clear error message (or return `'/'` depending on desired semantics).

This preserves the current browser behavior while preventing surprising crashes in non-browser callers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}