refactor for clarity

This commit is contained in:
Logan Houp 2024-04-05 14:01:53 -04:00
parent 0b3180070c
commit 36e77f8cf9

285
index.md

@ -11,36 +11,23 @@ toc:
ordered: true
---
<h1><img src="./regexide.png" style="height: 4rem; display: inline-block" alt="Regexide Logo"/></h1>
<img src="./regexide.png" style="height: 4rem; display: inline-block" alt="Regexide Logo"/>
This story begins with a simple question:
<h1>Denials of Service in Regular Expression</h1>
!!! question How do I remove XML comments in JavaScript?
This whole situation began with a simple question, "How do I remove XML comments in JavaScript?". The Internet hivemind converged on one general approach: <b>regular expressions</b>.
The Internet hivemind converged on one general approach: <b>regular expressions</b>.
This has one big problem. If you're not careful, parsing relatively small amounts of data could lead to browsers or servers freezing for extended periods of time. The official category for this weakness is "CWE-1333"[^5] "Inefficient Regular Expression Complexity". Some resources also use the phrase "Catastrophic backtracking" to describe the issue.
The most frequently recommended answer is:
```js
str = str.replace(/<!--[\s\S]*?-->/g, ""); // bad, do not use
^^^^^^^^^^^^^^^^^^
```
**There are known flaws with this family of regular expressions.**
This discussion focuses on "Regexide", the act of identifying and replacing flawed regular expressions with other techniques that better reflect the intended effect.
This discussion focuses on what we've come to call "Regexide", which we've defined as the act of identifying and replacing flawed regular expressions with other techniques that better reflect the intended effect. Let's look at a few examples.
[TOC]
## Why XML Comments matter
## How the Regular Expression Works
XML is a popular format for storing and sharing data. It was explicitly designed for people and programs to read and write data.[^1] From spreadsheets to save states, most modern software and games parse and write XML.
For the purposes of this discussion, it's important to understand exactly what XML comments are. XML comments are special notes that parsers should not treat as data. XML comments start with `<!--` and end with `-->`. Technically XML comments must not contain the string `--` within the comment body. Many programs and people write invalid XML comments, so parsers will typically allow for nested `--`.
XML comments are special notes that parsers should not treat as data. XML comments start with `<!--` and end with `-->`.
Technically XML comments must not contain the string `--` within the comment body. Many programs and people write invalid XML comments, so parsers will typically allow for nested `--`.
The following XML comment is technically invalid but accepted by many parsers:
For example, the following XML comment is technically invalid but accepted by many parsers:
```xml
<!-- I used to be a programmer like you,
@ -49,8 +36,6 @@ The following XML comment is technically invalid but accepted by many parsers:
(Kleene wrote a seminal paper[^2] on regular expressions.)
## How the regular expression works
The regular expression body `/<!--[\s\S]*?-->/` has three parts:
A) `<!--` matches the four literal characters
@ -90,32 +75,37 @@ str = str.replace(/<!--.*?-->/gs, ""); // even worse
The `/s` flag modifies the `.` character class to include line terminators.
### Usage in Open Source Projects
## Usage in Open Source Projects
Many popular open source projects use problematic regular expressions.
Many popular open source projects use problematic regular expressions. The most frequently recommended answer is:
```js
str = str.replace(/<!--[\s\S]*?-->/g, ""); // bad, do not use
^^^^^^^^^^^^^^^^^^
```
[Nunjucks](https://github.com/mozilla/nunjucks/blob/ea0d6d5396d39d9eed1b864febb36fbeca908f23/nunjucks/src/filters.js#L491) used this regular expression within in the `striptags` filter expression:
```js
let tags = /<\/?([a-z][a-z0-9]*)\b[^>]*>|<!--[\s\S]*?-->/gi;
let tags = /<\/?([a-z][a-z0-9]*)\b[^>]*>|<!--[\s\S]*?-->/gi
```
[PrettierJS](https://github.com/prettier/prettier/blob/45ad4668ebc133621c7f94e678ce399cab318068/scripts/lint-changelog.js#L51) used this regular expression in the build sequence:
```js
const templateComments = template.match(/<!--.*?-->/gs);
const templateComments = template.match(/<!--.*?-->/gs)
```
[RollupJS](https://github.com/rollup/rollup/blob/18372035f167ec104280e1e91ef795e4f7033f1e/scripts/release-helpers.js#L76) used this regular expression in the build sequence:
```js
const bodyWithoutComments = data.body.replace(/<!--[\S\s]*?-->/g, '');
const bodyWithoutComments = data.body.replace(/<!--[\S\s]*?-->/g, "")
```
[SheetJS](https://github.com/SheetJS/sheetjs/blob/master/xlsx.mjs#L18117) used this regular expression in parsing:
```js
str = str.replace(/<!--([\s\S]*?)-->/mg,"");
str = str.replace(/<!--([\s\S]*?)-->/gm, "")
```
[ViteJS](https://github.com/vitejs/vite/blob/9fc5d9cb3a1b9df067e00959faa9da43ae03f776/packages/vite/src/node/optimizer/scan.ts#L259) used the nascent `s` flag to ensure `.` matches newline characters:
@ -123,16 +113,16 @@ str = str.replace(/<!--([\s\S]*?)-->/mg,"");
```js
export const commentRE = /<!--.*?-->/gs
// Avoid matching the content of the comment
raw = raw.replace(commentRE, '<!---->')
// Avoid matching the content of the comment
raw = raw.replace(commentRE, "<!---->")
```
[VueJS 2](https://github.com/vuejs/vue/blob/v2.2.3/dist/vue.esm.js#L7404) used regular expressions in processing:
```js
text = text
.replace(/<!--([\s\S]*?)-->/g, '$1')
.replace(/<!\[CDATA\[([\s\S]*?)]]>/g, '$1');
.replace(/<!--([\s\S]*?)-->/g, "$1")
.replace(/<!\[CDATA\[([\s\S]*?)]]>/g, "$1")
```
[WordPress](https://github.com/WordPress/WordPress/blob/master/wp-admin/js/word-count.js#L73) used regular expressions in the word count calculator:
@ -144,24 +134,13 @@ text = text
[Element Plus](https://github.com/element-plus/element-plus/blob/4ac4750158fa634aa9da186111bce86c2898fda2/internal/build/src/tasks/helper.ts#L60) used a similar regular expression to match blocks starting with `<del>` and ending with `</del>`:
```js
const str = removeTag(value)
.replaceAll(/<del>.*<\/del>/g, '')
// ---------^^^^^^^^^^^^^^^^^^ -- start <del> end </del>
const str = removeTag(value).replaceAll(/<del>.*<\/del>/g, "")
// ---------^^^^^^^^^^^^^^^^^^ -- start <del> end </del>
```
### A rare consensus
## A Troubling Consensus
Most resources recommend this approach.
**Books** recommend this approach. "Regular Expressions Cookbook"[^3] section 9.9 explicitly recommends `/<!--[\s\S]*?-->/` for matching XML comments.
**StackOverflow Answers** recommend this regular expression and variants such as `/<!--[\s\S\n]*?-->/` (which are, for all practical purposes, equivalent).
**ChatGPT4** has recommended the previous regular expression. It also generated code for a complete unrelated tag.
**Bing AI** proposed unrelated command line tools for JavaScript.
<details><summary><b>ChatGPT4 and Bing AI Screenshots</b> (click to show)</summary>
It's surprising to see that most resources recommend this approach. A prominent O'Reilly textbook, "Regular Expressions Cookbook"[^3], explicitly recommends `/<!--[\s\S]*?-->/` in section 9.9 for matching XML comments. **StackOverflow Answers** recommend this regular expression and variants such as `/<!--[\s\S\n]*?-->/` (which are, for all practical purposes, equivalent). **ChatGPT4** has also recommended the previous regular expression. It also generated code for a complete unrelated tag. 🙄
_ChatGPT4 Incorrect interpretation_
@ -171,29 +150,33 @@ _ChatGPT4 Correct interpretation, solution uses vulnerable regular expression_
![ChatGPT correct interpretation](chatgpt.png)
**Bing AI** proposed unrelated command line tools for JavaScript.
<details><summary><b>ChatGPT4 and Bing AI Screenshots</b> (click to show)</summary>
_Bing AI Correct Interpretation, solution uses vulnerable regular expression_
![Bing AI correct interpretation](bing.png)
</details>
## The Internet Failed Us
## But Why is it So Slow?
There are deep performance issues with the regular expression. To see this, consider a string that repeats the header part `<!--` many times. In general, this type of string can be generated in JavaScript using `String#repeat`:
Consider a string that repeats the header part `<!--` many times. In general, this type of string can be generated in JavaScript using `String.prototype.repeat`:
```js
var string_repeated_65536_times = "<!--".repeat(65536);
var string_repeated_65536_times = "<!--".repeat(65536)
```
The `replace` operation is surprisingly slow. Try the following snippet in a new browser window or NodeJS terminal:
```js
// this loop doubles each time
for(var n = 64; n < 1000000; n*=2) {
var s = "<!--".repeat(n); // generate repeated string
console.time(n);
s.replace(/<!--([\s\S]*?)-->/mg,""); // replace
console.timeEnd(n);
for (var n = 64; n < 1000000; n *= 2) {
var s = "<!--".repeat(n) // generate repeated string
console.time(n)
s.replace(/<!--([\s\S]*?)-->/gm, "") // replace
console.timeEnd(n)
}
```
@ -205,41 +188,35 @@ Results are from local tests on a 2019 Intel i9 MacBook Pro. The following chart
When the number of repetitions doubled, the runtime roughly quadrupled. This is a "quadratic" relationship.
### Why the regular expression is slow
The regular expression matches a string that starts with `<!--` and ends with `-->`. Consider a function that repeatedly looks for the `<!--` string and tries to find the first `-->` that appears afterwards. Computer scientists classify this algorithm as "Backtracking"[^4]:
```js {.line-numbers}
function match_all_regex_comments(str) {
const results = [];
const results = []
/* look for the first instance of <!-- */
let start_index = str.indexOf("<!--");
let start_index = str.indexOf("<!--")
/* this loop runs while start_index is valid */
while(start_index > -1) {
while (start_index > -1) {
/* look for the first instance of --> after <!-- */
let end_index = str.indexOf("-->", start_index + 4);
let end_index = str.indexOf("-->", start_index + 4)
/* if --> is found, then we have a match! */
if(end_index > -1) {
if (end_index > -1) {
/* add to array */
results.push(str.slice(start_index, end_index + 3));
results.push(str.slice(start_index, end_index + 3))
/* start scanning from the end of the `-->` */
start_index = str.indexOf("<!--", end_index + 3);
}
else {
start_index = str.indexOf("<!--", end_index + 3)
} else {
/* jump to the next potential starting point */
start_index = str.indexOf("<!--", start_index + 1);
start_index = str.indexOf("<!--", start_index + 1)
}
}
/* return the final list */
return results;
return results
}
```
@ -251,6 +228,55 @@ function match_all_regex_comments(str) {
It can be shown that the runtime complexity of the modified algorithm is $\Theta(L+M)$ where $L$ is the string length and $M$ is the number of matches
### A Side Note About Rust
Everyone writes high-performance code in Rust, right? Rust does not have built-in support for regular expressions. The Rust `regress`[^6] crate is designed for JavaScript regular expressions. It represents a true apples-to-apples comparison with JavaScript. `regress` shows the same quadratic behavior as other JavaScript regular expression engines.
```rust
let re = regress::Regex::new(r"<!--([\s\S]*?)-->").unwrap();
let mut str = "<!--<!--<!--";
let _match = re.find(str);
```
<details><summary><b>Complete Example</b> (click to show)</summary>
```rust {.line-numbers}
fn main() {
let re = regress::Regex::new(r"<!--([\s\S]*?)-->").unwrap();
/* construct string by repeating with itself */
let mut str = "<!--";
let mut _str = format!("{}{}", str, str);
let mut rept: u64 = 1;
for _i in 1..8 {
_str = format!("{}{}", str, str);
str = _str.as_str();
rept *= 2;
}
for _j in 1..11 {
/* test regular expression against string */
let start_time = std::time::Instant::now();
let _caps = re.find(str);
let elapsed_time = start_time.elapsed();
println!("{}: {:?}", rept, elapsed_time);
/* double string length by repeating with itself */
_str = format!("{}{}", str, str);
str = _str.as_str();
rept *= 2;
}
}
```
<img src="./data/regress.png" style="max-height:200px" alt="rust regress performance test - quadratic complexity"/>
<p>Results are from local tests on a 2019 Intel i9 MacBook Pro. </p>
[Download the raw data as a CSV](./data/regress.csv)
</details>
### A Closer Look at the Math
If `-->` is not in the string, the scan `str.indexOf("-->", start_index + 4)` will look at every character in the string starting from `start_index + 4`. In the worst case, with repeated `<!--`, the scan will start from index `4`, then index `8`, then index `12`, etc.
The following diagram shows the first three scans when running the function against the string formed by repeating `<!--` 5 times. The `<!--` matches are highlighted in yellow and the scans for the `-->` are highlighted in blue.
@ -296,68 +322,6 @@ $$
In the worst case, the number of characters scanned is roughly proportional to the square of the length of the string. In "Big-O Notation", the complexity is $O(L^2)$. This is colloquially described as a "quadratic blowup".
### Vulnerability
This is generally considered a vulnerability since relatively small data can cause browsers or servers to freeze for extended periods of time.
The official category for this weakness is "CWE-1333"[^5] "Inefficient Regular Expression Complexity".
Some resources use the phrase "Catastrophic backtracking" to describe the issue.
### A side note about Rust
Everyone writes high-performance code in Rust, right?
Rust does not have built-in support for regular expressions. Third-party libraries fill the gap.
The Rust `regress`[^6] crate is designed for JavaScript regular expressions. It represents a true apples-to-apples comparison with JavaScript.
```rust
let re = regress::Regex::new(r"<!--([\s\S]*?)-->").unwrap();
let mut str = "<!--<!--<!--";
let _match = re.find(str);
```
<details><summary><b>Complete Example</b> (click to show)</summary>
```rust {.line-numbers}
fn main() {
let re = regress::Regex::new(r"<!--([\s\S]*?)-->").unwrap();
/* construct string by repeating with itself */
let mut str = "<!--";
let mut _str = format!("{}{}", str, str);
let mut rept: u64 = 1;
for _i in 1..8 {
_str = format!("{}{}", str, str);
str = _str.as_str();
rept *= 2;
}
for _j in 1..11 {
/* test regular expression against string */
let start_time = std::time::Instant::now();
let _caps = re.find(str);
let elapsed_time = start_time.elapsed();
println!("{}: {:?}", rept, elapsed_time);
/* double string length by repeating with itself */
_str = format!("{}{}", str, str);
str = _str.as_str();
rept *= 2;
}
}
```
</details>
Results are from local tests on a 2019 Intel i9 MacBook Pro. `regress` shows the same quadratic behavior as other JavaScript regular expression engines.
<img src="./data/regress.png" style="max-height:200px" alt="rust regress performance test - quadratic complexity"/>
[Download the raw data as a CSV](./data/regress.csv)
## Workarounds
There are a few general approaches to address the issue.
@ -373,19 +337,19 @@ The `re2`[^7] C++ engine sacrifices backreference and lookaround support for per
The `re2`[^8] NodeJS package is a native binding to the C++ engine and can be used in server-side environments. With modern versions of NodeJS, normal regular expressions can be wrapped with `RE2`:
```js
var out = str.replace(new RE2(/<!--([\s\S]*?)-->/mg),""); // replace
var out = str.replace(new RE2(/<!--([\s\S]*?)-->/gm), "") // replace
```
<details><summary><b>Complete Example</b> (click to show)</summary>
```js
var RE2 = require("re2");
var RE2 = require("re2")
// this loop doubles each time
for(var n = 64; n < 100000000; n*=2) {
var s = "<!--".repeat(n); // generate repeated string
console.time(n);
s.replace(new RE2(/<!--([\s\S]*?)-->/mg),""); // replace
console.timeEnd(n);
for (var n = 64; n < 100000000; n *= 2) {
var s = "<!--".repeat(n) // generate repeated string
console.time(n)
s.replace(new RE2(/<!--([\s\S]*?)-->/gm), "") // replace
console.timeEnd(n)
}
```
@ -440,8 +404,6 @@ fn main() {
}
```
</details>
The Rust `regex` implementation uses algorithms whose performance scales linearly with the size of the input.
<img src="./data/regex.png" style="max-height:200px" alt="rust regex performance test - linear complexity"/>
@ -466,7 +428,7 @@ The XML 1.0 specification[^10] disallows `--` within comments.
[PrettierJS](https://github.com/prettier/prettier/blob/ff83d55d05e92ceef10ec0cb1c0272ab894a00a0/src/language-markdown/mdx.js#L28) uses a regular expression in the MDX parser that enforces the XML constraint:
```js
const COMMENT_REGEX = /<!---->|<!---?[^>-](?:-?[^-])*-->/;
const COMMENT_REGEX = /<!---->|<!---?[^>-](?:-?[^-])*-->/
```
Commonly-used regular expression engines can optimize for this pattern and avoid backtracking.
@ -477,7 +439,6 @@ Commonly-used regular expression engines can optimize for this pattern and avoid
The XML parser in Excel powering the [Excel 2003-2004 (SpreadsheetML) format](https://docs.sheetjs.com/docs/miscellany/formats#excel-2003-2004-spreadsheetml) allows `--` in the comment body.
#### HTML Comments
The HTML5 standard[^11] permits `--` but forbids `<!--` within comment text. For example, the following comment is not valid according to the standard:
@ -498,7 +459,6 @@ This expression allows `--` but disallows `<!--` in the comment body. In practic
&lt;!-- I used to be a programmer like you, then I took an <span style="background-color: #FFFF00">&lt;!-- in the Kleene --&gt;</span>
</pre>
!!! info Web Browsers
Web browsers generally allow `<!--` in comments. Text between the first `<!--` and the first `-->` are treated as a comment. For example, consider the following HTML:
@ -522,49 +482,48 @@ Regular expression operations can be reimplemented using standard string operati
For example, the replacement
```js
str = str.replace(/<!--([\s\S]*?)-->/, "");
str = str.replace(/<!--([\s\S]*?)-->/, "")
```
can be rewritten with a loop. The core idea is to collect non-commented fragments:
```js {.line-numbers}
function remove_xml_comments(str) {
const START = "<!--", END = "-->";
const results = [];
const START = "<!--",
END = "-->"
const results = []
/* this index tracks the last analyzed character */
let last_index = 0;
let last_index = 0
/* look for the first instance of <!-- */
let start_index = str.indexOf(START);
let start_index = str.indexOf(START)
/* this loop runs while start_index is valid */
while(start_index > -1) {
while (start_index > -1) {
/* add the fragment that precedes the comment */
results.push(str.slice(last_index, start_index));
last_index = start_index;
results.push(str.slice(last_index, start_index))
last_index = start_index
/* look for the first instance of --> after <!-- */
let end_index = str.indexOf(END, start_index + START.length);
let end_index = str.indexOf(END, start_index + START.length)
/* if --> is found, then we have a match! */
if(end_index > -1) {
if (end_index > -1) {
/* skip the comment */
last_index = end_index + END.length;
last_index = end_index + END.length
/* search for next comment open tag */
start_index = str.indexOf(START, last_index);
}
start_index = str.indexOf(START, last_index)
} else break
/* if there is no end comment tag, stop processing */
else break;
}
/* add remaining part of string */
results.push(str.slice(last_index));
results.push(str.slice(last_index))
/* concatenate the fragments */
return results.join("");
return results.join("")
}
```