Skip to content

Fix length check for Regex BOL FindFirstChar optimization #55574

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

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

stephentoub
Copy link
Member

For a beginning-of-line anchor, in FindFirstChar we use IndexOf to quickly skip ahead to the next \n. But we neglected to check to see whether that brought us past an explicitly specified end position. This just adds the missing check.

Fixes #55557
cc: @strobelleder, @dotnet/area-system-text-regularexpressions

For a beginning-of-line anchor, in FindFirstChar we use IndexOf to quickly skip ahead to the next \n.  But we neglected to check to see whether that brought us past an explicitly specified end position.  This just adds the missing check.
@ghost
Copy link

ghost commented Jul 13, 2021

Tagging subscribers to this area: @eerhardt, @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

For a beginning-of-line anchor, in FindFirstChar we use IndexOf to quickly skip ahead to the next \n. But we neglected to check to see whether that brought us past an explicitly specified end position. This just adds the missing check.

Fixes #55557
cc: @strobelleder, @dotnet/area-system-text-regularexpressions

Author: stephentoub
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: -

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM

Do you think this warrants a backport to 5.0?

@stephentoub
Copy link
Member Author

Do you think this warrants a backport to 5.0?

It's a simple fix, so I'm not against backporting it. But it also requires a pretty special combination of things (a ^ anchor with RegexOptions.Multiline and specifying an end position to the match that's before a \n), so I don't expect it'll impact too many folks.

Copy link

@pgovind pgovind left a comment

Choose a reason for hiding this comment

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

LGTM

@stephentoub stephentoub merged commit ce09822 into dotnet:main Jul 13, 2021
@stephentoub stephentoub deleted the boloptfix branch July 13, 2021 21:10
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regex.Match(string, int, int) throws an exception when using a pattern with '^' or '$' and the Multiline option
3 participants