Skip to content

Correct escaping of regexp patterns created at run-time#2482

Merged
lahma merged 3 commits into
sebastienros:mainfrom
adams85:regexp-fixes-3
May 19, 2026
Merged

Correct escaping of regexp patterns created at run-time#2482
lahma merged 3 commits into
sebastienros:mainfrom
adams85:regexp-fixes-3

Conversation

@adams85

@adams85 adams85 commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Corrects escaping of regexp patterns created at run-time via new RegExp("\n"), RegExp("\n"), /x/.compile("\n"), "x".match("\n"), etc.

Details

The / character is not valid without escaping in regexp literals outside class sets (however, it's valid inside them). New line characters are not valid in regexp literals either. There are cases which aren't escaped correctly (e.g. new RegExp("[/]"), new RegExp("\r[\n]"), etc.) This PR aims to fix this.

Also, .NET Regex objects can be exposed to the engine. However, it's incorrect to return the .NET pattern as if it were a JS regexp pattern. This could be misleading. The PR suggests a placeholder (which is an invalid pattern BTW) instead: ?[native regex]. This is a breaking change though. I don't insist on it if you don't think it's a good idea.

Finally, makes a minor improvement to the Repl app: in interactive mode, it prints the regexp literal (like e.g. Chrome or Firefox dev tools do) instead of the meaningless {} text.

Linked issue

n/a

Test plan

  • Added or updated unit tests in Jint.Tests
  • Ran dotnet test --configuration Release locally
  • For ECMAScript spec changes: ran Jint.Tests.Test262 and confirmed no regressions
  • For interop changes: covered in Jint.Tests/Runtime/Interop
  • For perf changes: included before/after numbers from Jint.Benchmark

Breaking change?

Yes. Minor behavioral breaking change to JsRegExp.Source and ToString for native .NET Regex objects exposed to the engine, as explained above.

@lahma lahma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you!

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