codeant-ai-for-open-source[bot] commented on code in PR #35867:
URL: https://github.com/apache/superset/pull/35867#discussion_r3488507717
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts:
##########
@@ -89,6 +89,11 @@ export class OptionSelector {
[this.values[a], this.values[b]] = [this.values[b], this.values[a]];
}
+ reorder(from: number, to: number) {
+ const [moved] = this.values.splice(from, 1);
+ this.values.splice(to, 0, moved);
+ }
Review Comment:
**Suggestion:** The new reorder helper does not validate index bounds. If
drag metadata is stale or out of range, `splice` returns no element and
`undefined` gets inserted into the values list, which later causes runtime
failures when the UI treats each item as a column object. Add an early guard
for same-index and out-of-range `from`/`to` values before mutating the array.
[null pointer]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Explore Dnd column selector can crash on invalid indices.
- ⚠️ Drag-and-drop column reordering becomes unreliable under race
conditions.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the Explore view, perform a drag-and-drop reorder within the Dnd
column selector
control, which uses `DndColumnSelect` at
`superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx:42-116`.
`DndColumnSelect` registers `onShiftOptions` that calls
`optionSelector.reorder(dragIndex,
hoverIndex)` at lines 109-112.
2. The drag operation is handled by `ExploreDndContextProvider` in
`superset-frontend/src/explore/components/ExploreContainer/ExploreDndContext.tsx`.
On drag
end, `handleDragEnd` (lines 12-22, 190-22 in the same file) calls
`resolveDragEnd(active,
over)` (lines 112-153), which reads `active.data.current.dragIndex` and
`over.data.current.dragIndex` and, if both are numbers and types match,
invokes the
reorder callback at line 135.
3. The sortable items for the column selector are `OptionWrapper` instances
(`superset-frontend/src/explore/components/controls/DndColumnSelectControl/OptionWrapper.tsx:41-82`),
which set `data: { type, dragIndex: index, onShiftOptions }` into
`useSortable`. If, due
to stale drag metadata or a programming error, either `activeData.dragIndex`
or
`overData.dragIndex` becomes out of range with respect to
`optionSelector.values` (for
example, simulate this by calling `resolveDragEnd` in a unit test with
`dragIndex` equal
to 10 while `optionSelector.values.length` is 3), `resolveDragEnd` still
calls
`onShiftOptions(dragIndex, hoverIndex)` with these invalid indices at line
135.
4. `onShiftOptions` in `DndColumnSelect.tsx` (lines 109-113) forwards the
invalid indices
to `OptionSelector.reorder` in
`superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts:92-95`.
There, `this.values.splice(from, 1)` returns an empty array, so `moved`
becomes
`undefined`, and `this.values.splice(to, 0, moved)` inserts `undefined` into
`optionSelector.values`. On the next render, `valuesRenderer` (lines 117-158
in
`DndColumnSelect.tsx`) iterates `optionSelector.values` and passes each
`column` into
`OptionWrapper` and ultimately `StyledColumnOption`
(`OptionWrapper.tsx:116-124`), which
casts `column` as `ColumnMeta` and expects a valid object. When `column` is
`undefined`,
downstream chart-control components attempt to access properties on
`undefined`, causing a
runtime TypeError and breaking the column selector UI.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=13af4a0222ac4a039f55957ac73b6b62&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=13af4a0222ac4a039f55957ac73b6b62&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/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts
**Line:** 92:95
**Comment:**
*Null Pointer: The new reorder helper does not validate index bounds.
If drag metadata is stale or out of range, `splice` returns no element and
`undefined` gets inserted into the values list, which later causes runtime
failures when the UI treats each item as a column object. Add an early guard
for same-index and out-of-range `from`/`to` values before mutating the array.
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%2F35867&comment_hash=ab11dfbe41ae4478a416a86c572f00d08ae647df4de59e2d4c07d83d681a0ab1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35867&comment_hash=ab11dfbe41ae4478a416a86c572f00d08ae647df4de59e2d4c07d83d681a0ab1&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]
