Do not crash the dashboard on an invalid grouping URL parameter - #1110
Open
abefarris wants to merge 1 commit into
Open
Do not crash the dashboard on an invalid grouping URL parameter#1110abefarris wants to merge 1 commit into
abefarris wants to merge 1 commit into
Conversation
The grouping config is read from the ?grouping= query param and handed to
JSON.parse with no error handling, so a malformed or unexpected value
replaces the entire dashboard with an error boundary. Grouping is a
shareable-link surface, so these are reachable by anyone following a stale
or hand-edited URL, with no recovery except editing the URL:
?grouping={not-valid-json Expected property name or '}' in JSON...
[{"type":"banana"}] Unknown group type banana
{"panel":"nonsense"} n.filter is not a function
[{"type":null}] Unknown group type null
Treat the parameter as untrusted: wrap the parse, require an object of
arrays, and drop entries whose type is not a known grouping type. Anything
unusable is ignored and the panel falls back to its default grouping, which
is how the sibling `where` (filter) parameter already behaves.
Partially valid values keep their usable levels: [{type:null},{type:result}]
now renders flat results instead of failing.
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not crash the dashboard on an invalid grouping URL parameter
Branch:
fix-grouping-url-crash→develop· 1 commit ·ui/dashboard/src/hooks/useGroupingConfig.tsProblem
The
?grouping=URL parameter is parsed with a bareJSON.parseand no shapevalidation, so a malformed value takes down the whole dashboard with a white
screen. Four distinct fatal inputs:
?grouping=not-jsonSyntaxErrorfromJSON.parse?grouping="a string".filter is not a function?grouping={"panel":"nonsense"}.filter is not a function?grouping={"panel":[{"type":null}]}Unknown group type nulldeep in tree buildingAnyone can be handed a crashing link — a truncated paste of a shared URL is
enough. The sibling
where=(filter) parameter already tolerates junk; thisbrings grouping into line with it.
Fix
Wrap the parse in try/catch, validate the parsed shape (object of arrays), and
filter entries against the known grouping types. Unusable input degrades to the
default grouping instead of crashing.
Testing
Exercised all four inputs above plus valid configs in a live dashboard; junk
now renders the default view, valid params behave as before.