fix(ui): don't crash admin render on unparseable custom component route path#17046
Open
yashs33244 wants to merge 1 commit into
Open
fix(ui): don't crash admin render on unparseable custom component route path#17046yashs33244 wants to merge 1 commit into
yashs33244 wants to merge 1 commit into
Conversation
…te path A view path that path-to-regexp can't parse (e.g. a stray ':') threw an uncaught TypeError inside the route-matching find() callbacks, crashing the whole admin render. Catch the parse error and fall back to a literal path comparison so the bad pattern just doesn't match instead of taking down the panel.
yashs33244
requested review from
AlessioGr,
JarrodMFlesch and
jacobsfletch
as code owners
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
1 participant
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.
What?
When a custom admin component's route path can't be parsed by
path-to-regexp(for example a path containing a stray:),isPathMatchingRoutethrew an uncaughtTypeError: Missing parameter name at N. Because this runs inside the.find()callbacks that match custom-component routes, the throw crashed the entire admin server render. This change catches the parse error and falls back to a literal path comparison.Why?
This is the root cause behind #15241: with
@payloadcms/plugin-multi-tenant, any custom admin component (views,afterNavLinks,beforeNavLinks, ...) could surface a route path thatpath-to-regexp6.3.0 rejects, and a single bad pattern took down the whole panel withTypeError: Missing parameter name at 5. A pattern that can't be compiled should simply not match, not crash route matching for every page.How?
Wrapped the
pathToRegexp(...).exec(...)call inisPathMatchingRoutein atry/catch. On a parse error,matchisnull, so the existing fallback (viewRoute = viewPath) does a plain literal comparison: strict equality whenexact, and the existing segment-boundarystartsWithcheck otherwise. Because the fallback compares against the full literal pattern, it can't produce greedy false-positive matches. Added regression tests covering the previously-throwing inputs.Fixes #15241