Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update code to allow better static analysis #408

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update code to allow better static analysis #408

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 83 additions & 47 deletions src/parse/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,25 @@
/**
* Opening brace.
*/
function open() {
return match(/^{\s*/);
function open(): boolean {
const openMatch = /^{\s*/.exec(css);
if (openMatch) {
processMatch(openMatch);
return true;
}
return false;
}

/**
* Closing brace.
*/
function close() {
return match(/^}/);
const closeMatch = /^}/.exec(css);
if (closeMatch) {
processMatch(closeMatch);
return true;
}
return false;
}

/**
Expand All @@ -142,13 +152,9 @@
}

/**
* Match `re` and return captures.
* Update position and css string. Return the matches
*/
function match(re: RegExp) {
const m = re.exec(css);
if (!m) {
return;
}
function processMatch(m: RegExpExecArray) {
const str = m[0];
updatePosition(str);
css = css.slice(str.length);
Comment on lines 154 to 160
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the bulk of the changes. The rest is just adaptation for this internal API change.

Expand All @@ -159,7 +165,10 @@
* Parse whitespace.
*/
function whitespace() {
match(/^\s*/);
const m = /^\s*/.exec(css);
if (m) {
processMatch(m);
}
}

/**
Expand Down Expand Up @@ -187,10 +196,11 @@
return;
}

const m = match(/^\/\*[^]*?\*\//);
const m = /^\/\*[^]*?\*\//.exec(css);
if (!m) {
return error('End of comment missing');
}
processMatch(m);

return pos<CssCommentAST>({
type: CssTypes.comment,
Expand Down Expand Up @@ -231,10 +241,11 @@
* Parse selector.
*/
function selector() {
const m = match(/^([^{]+)/);
const m = /^([^{]+)/.exec(css);
if (!m) {
return;
}
processMatch(m);

// remove comment in selector;
let res = trim(m[0]).replace(commentre, '');
Expand Down Expand Up @@ -301,30 +312,41 @@
const pos = position();

// prop
const propMatch = match(/^(\*?[-#/*\\\w]+(\[[0-9a-z_-]+\])?)\s*/);
const propMatch = /^(\*?[-#/*\\\w]+(\[[0-9a-z_-]+\])?)\s*/.exec(css);
if (!propMatch) {
return;
}
processMatch(propMatch);
const propValue = trim(propMatch[0]);

// :
if (!match(/^:\s*/)) {
const sepratotorMatch = /^:\s*/.exec(css);
if (!sepratotorMatch) {
return error("property missing ':'");
}
processMatch(sepratotorMatch);

// val
const val = match(
/^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/,
);
let value = '';
const matchVal =
/^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with '(' and containing many repetitions of '""'.

Copilot Autofix AI 8 days ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that causes exponential backtracking. Specifically, we can refactor the pattern to avoid nested quantifiers and ambiguous alternations. The goal is to ensure that each part of the regular expression has a clear and unambiguous match.

In this case, we can break down the regular expression into smaller, more manageable parts and use non-capturing groups to avoid ambiguity. We will replace the problematic part of the regular expression with a more efficient version.

Suggested changeset 1
src/parse/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/parse/index.ts b/src/parse/index.ts
--- a/src/parse/index.ts
+++ b/src/parse/index.ts
@@ -331,3 +331,3 @@
     const matchVal =
-      /^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
+      /^((?:'(?:\\'|[^'\\])*'|"(?:\\"|[^"\\])*"|\((?:'(?:\\'|[^'\\])*'|"(?:\\"|[^"\\])*"|[^)])*\)|[^};])+)/.exec(
         css,
EOF
@@ -331,3 +331,3 @@
const matchVal =
/^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
/^((?:'(?:\\'|[^'\\])*'|"(?:\\"|[^"\\])*"|\((?:'(?:\\'|[^'\\])*'|"(?:\\"|[^"\\])*"|[^)])*\)|[^};])+)/.exec(
css,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with '('' and containing many repetitions of ''''.

Copilot Autofix AI 8 days ago

To fix the problem, we need to remove the ambiguity in the regular expression that causes exponential backtracking. Specifically, we should modify the sub-expression (?:\\'|.)*? to ensure that it does not have overlapping matches. We can achieve this by explicitly excluding the single quote character from the . match using a negated character class.

  • Modify the regular expression on line 332 to replace (?:\\'|.)*? with (?:\\'|[^'])*?.
  • This change ensures that the . match does not include the single quote character, thus removing the ambiguity and preventing exponential backtracking.
Suggested changeset 1
src/parse/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/parse/index.ts b/src/parse/index.ts
--- a/src/parse/index.ts
+++ b/src/parse/index.ts
@@ -331,3 +331,3 @@
     const matchVal =
-      /^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
+      /^((?:'(?:\\'|[^'])*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|[^'])*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
         css,
EOF
@@ -331,3 +331,3 @@
const matchVal =
/^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
/^((?:'(?:\\'|[^'])*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|[^'])*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
css,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options

Check failure

Code scanning / CodeQL

Inefficient regular expression High

This part of the regular expression may cause exponential backtracking on strings starting with '("' and containing many repetitions of '""'.

Copilot Autofix AI 8 days ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that causes exponential backtracking. Specifically, we should replace the sub-expression (?:\\"|.)*? with a more efficient pattern that avoids ambiguity. One way to achieve this is to use a character class that excludes the characters we want to match separately, thus eliminating the need for nested quantifiers.

In this case, we can replace (?:\\"|.)*? with [^"\\]* to match any sequence of characters that are not double quotes or backslashes. This change will ensure that the regular expression performs efficiently without changing its functionality.

Suggested changeset 1
src/parse/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/parse/index.ts b/src/parse/index.ts
--- a/src/parse/index.ts
+++ b/src/parse/index.ts
@@ -331,3 +331,3 @@
     const matchVal =
-      /^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
+      /^((?:'(?:\\'|[^'])*?'|"(?:\\"|[^"])*?"|\((?:'(?:\\'|[^'])*?'|"(?:\\"|[^"])*?"|[^)])*?\)|[^};])+)/.exec(
         css,
EOF
@@ -331,3 +331,3 @@
const matchVal =
/^((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|\((?:'(?:\\'|.)*?'|"(?:\\"|.)*?"|[^)])*?\)|[^};])+)/.exec(
/^((?:'(?:\\'|[^'])*?'|"(?:\\"|[^"])*?"|\((?:'(?:\\'|[^'])*?'|"(?:\\"|[^"])*?"|[^)])*?\)|[^};])+)/.exec(
css,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
css,
);
Comment on lines +332 to +334

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
Loading
that depends on
library input
Loading
may run slow on strings starting with '(' and with many repetitions of '"'.
This
regular expression
Loading
that depends on
library input
Loading
may run slow on strings starting with '('' and with many repetitions of '''.
This
regular expression
Loading
that depends on
library input
Loading
may run slow on strings starting with '("' and with many repetitions of '"'.
if (matchVal) {
value = trim(processMatch(matchVal)[0]).replace(commentre, '');
}

const ret = pos<CssDeclarationAST>({
type: CssTypes.declaration,
property: propValue.replace(commentre, ''),
value: val ? trim(val[0]).replace(commentre, '') : '',
value: value,
});

// ;
match(/^[;\s]*/);
const endMatch = /^[;\s]*/.exec(css);
if (endMatch) {
processMatch(endMatch);
}

return ret;
}
Expand Down Expand Up @@ -363,9 +385,13 @@
const vals = [];
const pos = position();

while ((m = match(/^((\d+\.\d+|\.\d+|\d+)%?|[a-z]+)\s*/))) {
vals.push(m[1]);
match(/^,\s*/);
while ((m = /^((\d+\.\d+|\.\d+|\d+)%?|[a-z]+)\s*/.exec(css))) {
const res = processMatch(m);
vals.push(res[1]);
const spacesMatch = /^,\s*/.exec(css);
if (spacesMatch) {
processMatch(spacesMatch);
}
}

if (!vals.length) {
Expand All @@ -384,19 +410,19 @@
*/
function atkeyframes(): CssKeyframesAST | void {
const pos = position();
const m1 = match(/^@([-\w]+)?keyframes\s*/);
const m1 = /^@([-\w]+)?keyframes\s*/.exec(css);

if (!m1) {
return;
}
const vendor = m1[1];
const vendor = processMatch(m1)[1];

// identifier
const m2 = match(/^([-\w]+)\s*/);
const m2 = /^([-\w]+)\s*/.exec(css);
if (!m2) {
return error('@keyframes missing name');
}
const name = m2[1];
const name = processMatch(m2)[1];

if (!open()) {
return error("@keyframes missing '{'");
Expand Down Expand Up @@ -426,12 +452,12 @@
*/
function atsupports(): CssSupportsAST | void {
const pos = position();
const m = match(/^@supports *([^{]+)/);
const m = /^@supports *([^{]+)/.exec(css);

if (!m) {
return;
}
const supports = trim(m[1]);
const supports = trim(processMatch(m)[1]);

if (!open()) {
return error("@supports missing '{'");
Expand All @@ -455,11 +481,12 @@
*/
function athost() {
const pos = position();
const m = match(/^@host\s*/);
const m = /^@host\s*/.exec(css);

if (!m) {
return;
}
processMatch(m);

if (!open()) {
return error("@host missing '{'");
Expand All @@ -482,12 +509,12 @@
*/
function atcontainer(): CssContainerAST | void {
const pos = position();
const m = match(/^@container *([^{]+)/);
const m = /^@container *([^{]+)/.exec(css);

if (!m) {
return;
}
const container = trim(m[1]);
const container = trim(processMatch(m)[1]);

if (!open()) {
return error("@container missing '{'");
Expand All @@ -511,15 +538,18 @@
*/
function atlayer(): CssLayerAST | void {
const pos = position();
const m = match(/^@layer *([^{;@]+)/);
const m = /^@layer *([^{;@]+)/.exec(css);

if (!m) {
return;
}
const layer = trim(m[1]);
const layer = trim(processMatch(m)[1]);

if (!open()) {
match(/^[;\s]*/);
const m2 = /^[;\s]*/.exec(css);
if (m2) {
processMatch(m2);
}
return pos<CssLayerAST>({
type: CssTypes.layer,
layer: layer,
Expand All @@ -544,12 +574,12 @@
*/
function atmedia(): CssMediaAST | void {
const pos = position();
const m = match(/^@media *([^{]+)/);
const m = /^@media *([^{]+)/.exec(css);

if (!m) {
return;
}
const media = trim(m[1]);
const media = trim(processMatch(m)[1]);

if (!open()) {
return error("@media missing '{'");
Expand All @@ -573,15 +603,16 @@
*/
function atcustommedia(): CssCustomMediaAST | void {
const pos = position();
const m = match(/^@custom-media\s+(--\S+)\s*([^{;\s][^{;]*);/);
const m = /^@custom-media\s+(--\S+)\s*([^{;\s][^{;]*);/.exec(css);

Check failure

Code scanning / CodeQL

Polynomial regular expression used on uncontrolled data High

This
regular expression
Loading
that depends on
library input
Loading
may run slow on strings starting with '@custom-media --!!' and with many repetitions of '!!'.
if (!m) {
return;
}
const res = processMatch(m);

return pos<CssCustomMediaAST>({
type: CssTypes.customMedia,
name: trim(m[1]),
media: trim(m[2]),
name: trim(res[1]),
media: trim(res[2]),
});
}

Expand All @@ -590,10 +621,11 @@
*/
function atpage(): CssPageAST | void {
const pos = position();
const m = match(/^@page */);
const m = /^@page */.exec(css);
if (!m) {
return;
}
processMatch(m);

const sel = selector() || [];

Expand Down Expand Up @@ -625,13 +657,14 @@
*/
function atdocument(): CssDocumentAST | void {
const pos = position();
const m = match(/^@([-\w]+)?document *([^{]+)/);
const m = /^@([-\w]+)?document *([^{]+)/.exec(css);
if (!m) {
return;
}
const res = processMatch(m);

const vendor = trim(m[1]);
const doc = trim(m[2]);
const vendor = trim(res[1]);
const doc = trim(res[2]);

if (!open()) {
return error("@document missing '{'");
Expand All @@ -656,10 +689,11 @@
*/
function atfontface(): CssFontFaceAST | void {
const pos = position();
const m = match(/^@font-face\s*/);
const m = /^@font-face\s*/.exec(css);
if (!m) {
return;
}
processMatch(m);

if (!open()) {
return error("@font-face missing '{'");
Expand Down Expand Up @@ -688,10 +722,11 @@
*/
function atstartingstyle(): CssStartingStyleAST | void {
const pos = position();
const m = match(/^@starting-style\s*/);
const m = /^@starting-style\s*/.exec(css);
if (!m) {
return;
}
processMatch(m);

if (!open()) {
return error("@starting-style missing '{'");
Expand Down Expand Up @@ -739,12 +774,13 @@

return function (): T1 | void {
const pos = position();
const m = match(re);
const m = re.exec(css);
if (!m) {
return;
}
const res = processMatch(m);
const ret: Record<string, string> = {type: name};
ret[name] = m[1].trim();
ret[name] = res[1].trim();
return pos<T1>(ret as unknown as T1) as T1;
};
}
Expand Down