Skip to content

Unskip ReDoS test#293

Open
timostamm wants to merge 1 commit intojonbodner/add_re2from
tstamm/Unskip-ReDoS-test
Open

Unskip ReDoS test#293
timostamm wants to merge 1 commit intojonbodner/add_re2from
tstamm/Unskip-ReDoS-test

Conversation

@timostamm
Copy link
Copy Markdown
Member

No description provided.

@timostamm timostamm requested a review from jonbodner-buf April 29, 2026 11:41
Copy link
Copy Markdown

@jonbodner-buf jonbodner-buf left a comment

Choose a reason for hiding this comment

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

One question about PR ordering.

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", () => {
Copy link
Copy Markdown

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?

Copy link
Copy Markdown

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants