rusackas opened a new pull request, #42202:
URL: https://github.com/apache/superset/pull/42202
### SUMMARY
`getOverrideHtmlSchema` in `SafeMarkdown.tsx` merges an operator's
`htmlSchemaOverrides`/`HTML_SANITIZATION_SCHEMA_EXTENSIONS` into
`rehype-sanitize`'s default schema using a `mergeWith` customizer that
concatenates attribute-definition arrays. hast-util-sanitize's `findDefinition`
only ever returns the *first* definition it finds for a given property name,
and the default schema already declares restrictive tuples for some tags, e.g.:
```js
li: [['className', 'task-list-item']],
ul: [...aria, ['className', 'contains-task-list']],
ol: [...aria, ['className', 'contains-task-list']],
```
So concatenating an operator's override *after* those defaults never
actually widened anything — the restrictive default tuple was always found
first. Worse, since a failed allowlist check on `className` returns `[]` (an
empty array) rather than `undefined`, hast-util-sanitize's own `'*'` wildcard
fallback never kicked in either. The net effect (see #34191): any CSS class set
on `<li>`/`<ul>`/`<ol>` in a Handlebars/Markdown chart gets silently stripped
to an empty `class=""`, even with `HTML_SANITIZATION_SCHEMA_EXTENSIONS`
configured to allow it.
This PR changes the merge so that, within `attributes`, an override
definition for a property **replaces** the default definition for that same
property (deduped by property name) instead of just being appended alongside
it. Other schema arrays (`tagNames`, `protocols.*`, etc.) keep the original
concat behavior.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — this is a schema-merge logic fix with no visual changes on its own;
see #34191 for screenshots of the bug (classes on `<li>`/`<ul>` being stripped
to `class=""`).
### TESTING INSTRUCTIONS
Added regression tests in
`superset-frontend/packages/superset-ui-core/test/components/SafeMarkdown.test.tsx`
(`getOverrideHtmlSchema` describe block) that:
- Pin the bug: with `htmlSchemaOverrides: { attributes: { li: ['className']
} }`, the merged schema's `li` definition must be replaced by the override, not
have the restrictive default tuple win.
- Pin the `'*'` wildcard case: with `htmlSchemaOverrides: { attributes: {
'*': ['className'] } }`, the wildcard override is present, and (documented for
clarity) an `li`-specific default still wins unless `li` itself is overridden —
since `findDefinition` checks the tag-specific list before `'*'`.
- Assert the default task-list `className` restriction on `li`/`ul`/`ol` is
unchanged when no override is supplied.
Manually:
1. Set `HTML_SANITIZATION_SCHEMA_EXTENSIONS = {"attributes": {"li":
["className"]}}` (or `"*": ["className"]` plus a per-tag override) in
`superset_config.py`.
2. Build a Handlebars or Markdown chart with an `<li class="my-class">`
element.
3. Confirm the rendered `<li>` keeps `class="my-class"` instead of being
stripped to `class=""`.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #34191
- [ ] 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]
