Skip to content

Unskip ReDoS test #293

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

Open
timostamm wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Unskip ReDoS test #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b788755
add re2js to codebase. change logic.ts to call it. fix some tests wit…
jonbodner-buf Apr 17, 2026
1872bae
fix imports
jonbodner-buf Apr 17, 2026
9a4a332
move re2 to its own package. add LICENCE and initial README.md
jonbodner-buf Apr 17, 2026
cbcc87b
update version in package-lock.json
jonbodner-buf Apr 17, 2026
e768b44
fix formatting. start fixing lint errors.
jonbodner-buf Apr 20, 2026
0eac393
additional lint updates
jonbodner-buf Apr 20, 2026
100f4db
finish lint changes
jonbodner-buf Apr 20, 2026
6554cea
add test to validate unicode range tables. fix delta ranges in Unicod…
jonbodner-buf Apr 20, 2026
dd34db0
fix formatting. remove unused methods, constants, and files.
jonbodner-buf Apr 21, 2026
cccca46
revert import change in checker.test.ts
jonbodner-buf Apr 21, 2026
1fc17e8
don't put license headers on the re2 files
jonbodner-buf Apr 21, 2026
68ce0a3
updates for PR
jonbodner-buf Apr 24, 2026
af39200
remove unneeded dependency from root package.json.
jonbodner-buf Apr 28, 2026
cea579f
Unskip ReDoS test
timostamm Apr 29, 2026
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
165 changes: 96 additions & 69 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@
"license-header": "license-header --ignore 'packages/**'",
"lint": "biome lint --error-on-warnings"
},
"workspaces": ["packages/cel-spec", "packages/cel", "packages/example"],
"workspaces": [
"packages/cel-spec",
"packages/cel",
"packages/example",
"packages/re2"
],
"type": "module",
"packageManager": "npm@10.9.0",
"licenseHeader": {
Expand Down
7 changes: 4 additions & 3 deletions packages/cel/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,12 @@
"@bufbuild/protobuf": "^2.6.2"
},
"dependencies": {
"@bufbuild/cel-spec": "0.4.0"
"@bufbuild/cel-spec": "0.4.0",
"@bufbuild/re2": "0.4.0"
},
"devDependencies": {
"expect-type": "^1.3.0",
"peggy": "^5.0.6",
"peggy-ts": "github:hudlow/peggy-ts#v0.0.9",
"expect-type": "^1.3.0"
"peggy-ts": "github:hudlow/peggy-ts#v0.0.9"
}
}
2 changes: 1 addition & 1 deletion packages/cel/src/std/logic.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function durationOf(func: () => void, times = 5): number {

void suite("logic", () => {
void suite("matches(string, string) -> bool", () => {
void test.skip("doesn't evaluate simple ReDoS expressions in exponential time", () => {
void test("doesn't evaluate simple ReDoS expressions in exponential time", () => {

@jonbodner-buf jonbodner-buf Apr 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we merge this before the re2js merge?

@jonbodner-buf jonbodner-buf Apr 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

apparently not, it depends on @bufbuild/re2

@timostamm timostamm Apr 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The test is a guard against ReDoS. It fails with the current implementation on main, and I expect it to pass in your branch.

It's currently skipped on main because it fails on main. We could merge this change into main, but it means CI would be red, and we'd be stuck if some other work comes along before we can merge your branch.

const maliciousRegex = "^(a*)*$";
const overhead = durationOf(() => matches.call("!", maliciousRegex));

Expand Down
Loading
Loading