sadpandajoe opened a new pull request, #42015:
URL: https://github.com/apache/superset/pull/42015
### SUMMARY
Two bugs in the legacy `/superset/*` → canonical route redirect shim
(`LegacyPrefixRedirectMiddleware`), both of which break real deployments.
**1. `HEAD` against any legacy route returns `410 Gone` instead of
redirecting.**
`LEGACY_REDIRECT_MAP` spells only the methods each canonical route's
`@expose`
decorator names explicitly, which never includes `HEAD` — Werkzeug registers
`HEAD` implicitly on every `GET` rule, so no route declares it. The
middleware
compared the raw request method against that map, so every legacy `HEAD`
request
fell into the wrong-method branch and got `410 Gone`. That breaks
link-checkers,
uptime monitors, and plain `curl -I` against any legacy URL.
`HEAD` is `GET`-without-a-body (RFC 9110 §9.3.2), so it is now folded to
`GET`
before the method check. It *tracks* `GET` rather than being blanket-allowed:
`HEAD` against the one `POST`-only canonical (`/log/`) still correctly
returns
`410`.
**2. The shim is disabled entirely when `APPLICATION_ROOT` is `/superset`.**
An `_enabled` guard turned the middleware off in that deployment, on the
premise
that the legacy prefix and the app-root prefix being the same token would
produce
an infinite 308 loop.
**That premise was incorrect.** The app-root strip runs *before* the
legacy-prefix
check, so a canonical URL can never re-enter the redirect branch: inbound
`/superset/welcome/` strips to `/welcome/`, which is not a legacy path, and
passes
through untouched. No redirect, no loop. The guard's only actual effect was
to
make legacy bookmarks under that root — which arrive carrying the prefix
twice,
`/superset/superset/dashboard/1/` — hard-404 rather than 308 to
`/superset/dashboard/1/`.
The guard is removed. In its place is a precise per-request check: if the
computed
redirect target is byte-identical to the inbound path, pass through instead
of
emitting a redirect that could not converge. No enumerated row can produce
that
today (no canonical target begins with the legacy prefix, so the rewrite
always
shortens the path) — it's a guard against a future map edit, not a live
condition.
### TESTING INSTRUCTIONS
`pytest tests/unit_tests/middleware/test_legacy_prefix_redirect.py` — 206
pass.
Both fixes were written test-first; the 58 new tests fail against the
previous
middleware and pass against this one.
- **HEAD:** new "Section A.0" parametrizes every `GET`-redirecting row over
`HEAD`,
with and without an app-root prefix, plus the SQL Lab deep-link special
case and
a `/log/` control asserting `HEAD` against a `POST`-only canonical still
`410`s.
- **App root:** new tests assert that under `APPLICATION_ROOT=/superset` a
doubled
legacy path 308s to the canonical one, a canonical path passes straight
through
(the no-loop invariant), and the bare app root is untouched. Four existing
tests
that pinned the buggy `_enabled` behavior were rewritten to pin the fixed
behavior.
Manual: deploy with `SUPERSET_APP_ROOT=/superset` and confirm
`curl -I <host>/superset/superset/dashboard/1/` returns `308` with
`Location: /superset/dashboard/1/` (both bugs at once — it was `410` before,
and `404` on the followed target).
### 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
--
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]
