Copilot commented on code in PR #39925:
URL: https://github.com/apache/superset/pull/39925#discussion_r3262763169
##########
superset/views/core.py:
##########
@@ -134,6 +134,16 @@
class Superset(BaseSupersetView):
"""The base views for Superset!"""
+ # Flask-AppBuilder's BaseView auto-derives `route_base` from the class
+ # name, which would mount every @expose route under `/superset/...`. That
+ # collides catastrophically with subdirectory deployments: AppRootMiddle-
+ # ware strips the configured root from PATH_INFO and sets SCRIPT_NAME, so
+ # url_for emits `/superset/superset/...` (doubled), and the in-rule
+ # `/superset` prefix no longer matches the post-strip PATH_INFO — making
+ # the routes themselves unreachable. Mount at the root so the application
+ # root applies exactly once via SCRIPT_NAME / basename.
+ route_base = ""
+
Review Comment:
`Superset.get_redirect_url()` still does a hard string replace of
`"/superset/explore" → "/explore"` and then returns a path-only URL. After
setting `route_base = ""`, the only `/superset` segment on subdirectory
deployments comes from `SCRIPT_NAME`, so this replace risks stripping the
application-root prefix and redirecting users to `/explore/...` (outside the
subdirectory). Consider rebuilding the redirect using
`url_for('ExploreView.root', **query)` (or otherwise preserving
`request.script_root`) instead of string replacement, so subdirectory redirects
remain single-prefixed.
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/DashboardsSubMenu.tsx:
##########
@@ -72,7 +72,7 @@ export const useDashboardsMenuItems = ({
<Link
target="_blank"
rel="noreferer noopener"
Review Comment:
`rel` is set to "noreferer noopener"; the correct token is `noreferrer`
(otherwise browsers ignore it and may still send a Referer header). Update to
`rel="noreferrer noopener"` (or at least ensure `noreferrer` is present).
##########
superset-frontend/src/features/home/SavedQueries.tsx:
##########
@@ -306,7 +305,7 @@ export const SavedQueries = ({
<CardStyles key={q.id}>
<ListViewCard
imgURL=""
- url={ensureAppRoot(`/sqllab?savedQueryId=${q.id}`)}
+ url={`/sqllab?savedQueryId=${q.id}`}
title={q.label}
Review Comment:
`ListViewCard` falls back to rendering a plain `<a href>` when
`linkComponent` isn’t provided (see superset-ui-core ListViewCard). Passing an
unprefixed absolute path like `/sqllab?...` will 404 under subdirectory
deployments (`SUPERSET_APP_ROOT`), because the anchor bypasses React Router’s
basename. Either pass `linkComponent={Link}` so routing/basename is applied, or
wrap the URL with `ensureAppRoot`/use `<AppLink>` so the href is correctly
prefixed.
##########
superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts:
##########
@@ -275,8 +275,23 @@ export default class SupersetClientClass {
const host = inputHost ?? this.host;
const cleanHost = host.slice(-1) === '/' ? host.slice(0, -1) : host; // no
backslash
+ // Strip a leading appRoot segment so callers that accidentally pre-prefix
+ // their endpoint (e.g. by wrapping with ensureAppRoot before passing to
the
+ // client) do not produce a doubled `/superset/superset/...` URL. The L2
+ // static invariant still flags this pattern as a migration issue; this is
+ // the runtime safety net.
+ let cleanEndpoint = endpoint;
+ const root = this.appRoot;
+ if (root) {
+ if (cleanEndpoint === root) {
+ cleanEndpoint = '';
+ } else if (cleanEndpoint.startsWith(`${root}/`)) {
+ cleanEndpoint = cleanEndpoint.slice(root.length);
+ }
Review Comment:
`getUrl()` strips at most one leading `appRoot` segment from `endpoint`. If
an upstream bug produces a doubly-prefixed endpoint (e.g.
`/superset/superset/api/...`), the method will still emit a doubled URL.
Consider making the stripping logic greedy (loop while `endpoint === root` or
starts with `${root}/`) similar to `stripAppRoot`, so the runtime safety-net
fully neutralizes repeated prefixes.
--
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]
