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.

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

fix: update regex engine to support possessive match and lookbehind syntaxes #1147

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cmmoran
Copy link

@cmmoran cmmoran commented Jan 31, 2024

Update the regexp to conditionally (naively?) change the current start,end delimiters for the regexp from the hard-coded '<','>' to '{','}' when the compiler encounters a pattern that has syntax that includes '(?<' or '(?>' organically.

BREAKING CHANGES: None

Related issue(s)

Relates:
#441

Fixes:
#1089

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    security@ory.sh) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

…ters if the regexp contains possessive match or lookbehind syntax.
@cmmoran cmmoran requested a review from aeneasr as a code owner January 31, 2024 18:15
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.88%. Comparing base (8fc9b7a) to head (ae8fb6e).
Report is 7 commits behind head on master.

Current head ae8fb6e differs from pull request most recent head e3e9566

Please upload reports for the commit e3e9566 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1147      +/-   ##
==========================================
- Coverage   77.90%   77.88%   -0.03%     
==========================================
  Files          80       80              
  Lines        3929     4047     +118     
==========================================
+ Hits         3061     3152      +91     
- Misses        595      616      +21     
- Partials      273      279       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aeneasr
Copy link
Member

aeneasr commented Sep 23, 2024

Thank you for the changes, however the reason lookbehind is not working is because we're using the golang std library, which does not support these features.

@aeneasr
Copy link
Member

aeneasr commented Sep 23, 2024

I'm actually wrong, looksl ike this is working, which is very nice!

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