codeant-ai-for-open-source[bot] commented on code in PR #40502:
URL: https://github.com/apache/superset/pull/40502#discussion_r3319478959
##########
superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts:
##########
@@ -146,24 +147,32 @@ function Rose(element: HTMLElement, props: RoseProps):
void {
function legendData(adatum: RoseData) {
return adatum[times[0]].map((v: RoseDataEntry, i: number) => ({
disabled: state.disabled[i],
- key: v.name,
+ // nvd3-fork's legend currently renders `key` via .text(), so raw
+ // markup would be escaped today. Sanitize at the data boundary
+ // anyway: it makes the safety property a local invariant rather
+ // than depending on the vendored legend's render choice.
+ key: sanitizeHtml(v.name),
Review Comment:
**Suggestion:** Sanitizing `v.name` before storing it in `legendData().key`
changes the series identity used by legend color mapping, while arcs still use
the raw `name` for `fill`. For values that are transformed by sanitization,
legend swatches and slice colors diverge (and distinct raw keys can collapse to
the same sanitized key). Keep a raw key for color/state identity and sanitize
only at the final HTML render sink. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Rose (VizType.Rose) legend colors diverge from arc colors.
- ⚠️ Users may misread which legend item controls which slice.
- ⚠️ Color collisions possible for distinct series after sanitization.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render a Rose chart (VizType.Rose) in Superset, which uses `new
RoseChartPlugin().configure({ key: VizType.Rose })` registered in
`superset-frontend/src/visualizations/presets/MainPreset.ts:23`.
2. The chart plugin loads `ReactRose` via `loadChart: () =>
import('./ReactRose')` in
`superset-frontend/plugins/legacy-plugin-chart-rose/src/index.ts:15-22`, and
`ReactRose`
in turn renders the `Rose` function from
`superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts:107-123`.
3. Ensure the query data for the chart includes at least one series name
containing
HTML-like markup (e.g., a group-by value `"<b>Category A</b>"` or a value
with disallowed
attributes) so that `queriesData[0].data` passed from `transformProps`
(`transformProps.ts:21-41`) yields `RoseDataEntry.name` values that will be
transformed by
`sanitizeHtml`.
4. On render, arcs are colored via `attr('fill', d => colorFn(d.name,
sliceId))` in
`Rose.ts:169-173`, using the RAW `name` as the color key, while the legend
entries are
built by `legendData()` (`Rose.ts:147-155`) which now sets `key:
sanitizeHtml(v.name)` at
line 154, and legend colors are assigned with `legend.width(width).color(d =>
colorFn(d.key, sliceId))` at `Rose.ts:188`.
5. Observe that for any series where `sanitizeHtml(v.name)` differs from
`v.name` (e.g.,
tags/attributes stripped or normalized), the legend's swatch color (based on
the sanitized
`key`) no longer matches the corresponding arc's fill color (based on the
raw `name`), and
in edge cases distinct raw names can collapse to the same sanitized `key`,
causing
multiple legend entries to share the same color even though their arcs are
different.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6aac4467c1064502981164cc89240993&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6aac4467c1064502981164cc89240993&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/legacy-plugin-chart-rose/src/Rose.ts
**Line:** 154:154
**Comment:**
*Logic Error: Sanitizing `v.name` before storing it in
`legendData().key` changes the series identity used by legend color mapping,
while arcs still use the raw `name` for `fill`. For values that are transformed
by sanitization, legend swatches and slice colors diverge (and distinct raw
keys can collapse to the same sanitized key). Keep a raw key for color/state
identity and sanitize only at the final HTML render sink.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40502&comment_hash=c6c6079f3ac1e74f9573ce5585a5488bcc4be082f52162ae17874a7dfca86c5b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40502&comment_hash=c6c6079f3ac1e74f9573ce5585a5488bcc4be082f52162ae17874a7dfca86c5b&reaction=dislike'>👎</a>
--
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]
