rusackas commented on code in PR #40546:
URL: https://github.com/apache/superset/pull/40546#discussion_r3342879277
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -246,7 +246,7 @@ export const SaveDatasetModal = ({
if (openWindow) {
window.open(url, '_blank', 'noreferrer');
} else {
- window.location.href = url;
+ window.location.href = sanitizeUrl(url);
}
Review Comment:
Good catch — fixed. `createWindow` now wraps the `window.open` branch with
`sanitizeUrl` too, so both navigation paths in this helper are consistently
sanitized (matching the `navigateTo` pattern).
##########
superset-frontend/src/features/databases/DatabaseModal/SqlAlchemyForm.tsx:
##########
@@ -87,7 +88,9 @@ const SqlAlchemyTab = ({
<div className="helper">
{t('Refer to the')}{' '}
<a
- href={fallbackDocsUrl || conf?.SQLALCHEMY_DOCS_URL || ''}
+ href={sanitizeUrl(
+ fallbackDocsUrl || conf?.SQLALCHEMY_DOCS_URL || '',
+ )}
target="_blank"
Review Comment:
The `fallback || conf?.SQLALCHEMY_DOCS_URL || ""` precedence is pre-existing
behavior that this PR did not change — it only wrapped the existing expression
in `sanitizeUrl` to harden the href sink. Reordering the precedence so `conf`
overrides the defaults would be a behavior change unrelated to the
URL-sanitization scope of this PR, so I am leaving it as-is here.
##########
superset-frontend/packages/superset-ui-core/src/components/ListViewCard/index.tsx:
##########
@@ -140,7 +141,7 @@ const ThinSkeleton = styled(Skeleton)`
const paragraphConfig = { rows: 1, width: 150 };
const AnchorLink: FC<LinkProps> = ({ to, children }) => (
- <a href={to}>{children}</a>
+ <a href={to !== undefined ? sanitizeUrl(to) : undefined}>{children}</a>
);
Review Comment:
The `to !== undefined` guard is intentional rather than dead code: although
`LinkProps.to` is typed as `string`, multiple callers pass `undefined` at
runtime to render non-clickable cards (e.g. bulk-select states). Sanitizing
unconditionally would turn an absent `href` into a string, so the guard
preserves the non-navigating behavior. Keeping it.
--
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]
