sadpandajoe opened a new pull request, #38316: URL: https://github.com/apache/superset/pull/38316
### SUMMARY `ensureAppRoot()` in `pathUtils.ts` unconditionally prepended the application root to every input, breaking navigation when `brandLogoHref` (set via `LOGO_TARGET_PATH`) is an absolute URL. The result was a double-prefixed URL like `https://<workspace>.us1a.app.preset.io/https://manage.app.preset.io`, producing a 404. **Root cause**: apache/superset#36058 (commit `e4f649e49c`) wrapped `theme.brandLogoHref` in `ensureAppRoot()` without guarding against absolute inputs. The same PR added `ensureStaticPrefix()`, which correctly passes absolute URLs through unchanged — `ensureAppRoot()` had no equivalent guard. **Fix**: `ensureAppRoot()` now detects absolute URLs via the RFC 3986 scheme pattern (`^[a-zA-Z][a-zA-Z0-9+\-.]*:`) and protocol-relative URLs (`//`), returning them unchanged. Relative paths continue to receive the application root prefix as before, preserving the subdirectory-deployment behavior introduced in #36058. `makeUrl()` inherits the fix automatically as it delegates to `ensureAppRoot()`. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — URL construction bug, no visual diff. **Before**: `ensureAppRoot('https://manage.app.preset.io')` → `https://<workspace>/https://manage.app.preset.io` (404) **After**: `ensureAppRoot('https://manage.app.preset.io')` → `https://manage.app.preset.io` ✓ ### TESTING INSTRUCTIONS ```bash # Run targeted tests npm run test -- src/utils/pathUtils.test.ts --maxWorkers=2 npm run test -- src/features/home/Menu.test.tsx --maxWorkers=2 ``` **Manual verification** (requires a deployment with `APPLICATION_ROOT` set to a subdirectory and `LOGO_TARGET_PATH` set to an absolute URL): 1. Configure `LOGO_TARGET_PATH = 'https://manage.app.preset.io'` (or any absolute URL) 2. Load the app — confirm the navbar logo `href` is the absolute URL, not prefixed with the workspace origin 3. Click the logo — confirm navigation goes to the correct destination 4. With a relative `LOGO_TARGET_PATH` (e.g. `/welcome`) confirm the app-root prefix is still applied ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API **Regression guardrails added in tests:** - `pathUtils.test.ts`: relative paths still receive the app-root prefix under `/superset/` - `Menu.test.tsx`: navbar brand `href` remains app-root prefixed for relative internal paths (PR #36058 intent preserved) 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
