fix(ast builder): literal list being coerced as string #1059
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
packages/app-builder/src/components/AstBuilder/edition/coerceToConstantAstNode.ts
Fixed
Show fixed
Hide fixed
ChibiBlasphem
force-pushed
the
fix/ast-builder/list-cercion
branch
from
8c48e24 to
a7d0a6e
Compare
|
|
||
| const isNumberArray = /^\[(\s*(\d+(\.\d+)?)\s*,?)*(\s*|\])$/; | ||
| const isStringArray = /^\[(\s*"?(\w+)"?\s*,?)*(\s*|\])$/; | ||
| const isStringArray = /^\[(\s*"?([^",[\]]+)"?\s*,?)*(\s*|\])$/; |
Check failure
Code scanning / CodeQL
Inefficient regular expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 2 months ago
To fix this issue, we need to rewrite the regular expression on line 48 to avoid ambiguity within repeated alternatives. The main concern is that ([^",[\]]+) appears inside a structure where the engine might consider multiple paths to match a substring of the input, especially if the input contains characters like [ at certain places.
The fix is to rewrite the inner pattern to make it unambiguous and avoid catastrophic backtracking. This can be done by using a non-capturing group, ensuring no overlapping, or being more precise in matching strings between delimiters. Since the intent is to match an array-like string, a safer alternative is to more narrowly match quoted strings or non-quote, non-bracket substrings using a non-greedy approach or atomic groups (ES2018+), or alternatively change + to *, ensuring it accepts empty fields as well. But the core of the fix is to rework [^",[\]]+ so that it cannot match the empty string or repeatedly lead to ambiguous parses.
One robust alternative for array string matching is to match only quoted substrings, or if supporting unquoted as well, make the branch for quoted and unquoted values separate and unambiguous. For now, a direct improvement, per the general advice, is to replace ([^",[\]]+) with ([^",[\]]*) and adjust the rest accordingly, or be even stricter.
Additionally, since we only have access to the file containing the regex definition, we only need to update the regex at line 48.
| @@ -45,7 +45,7 @@ | ||
| } | ||
|
|
||
| const isNumberArray = /^\[(\s*(\d+(\.\d+)?)\s*,?)*(\s*|\])$/; | ||
| const isStringArray = /^\[(\s*"?([^",[\]]+)"?\s*,?)*(\s*|\])$/; | ||
| const isStringArray = /^\[(\s*"?([^",[\]]*)"?\s*,?)*\s*\]$/; | ||
|
|
||
| const captureNumbers = /(?:\s*(?<numbers>\d+(\.\d+)?)\s*,?)/g; | ||
| const captureStrings = /(?:\s*"?(?<strings>[^",[\]]*[^",[\]])"?\s*,?)/g; |
2 participants
" , ] [)