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.

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

Jump to bottom

<regex>: add checks for integer overflow #2169

Merged
merged 18 commits into from
Aug 27, 2022
Merged

<regex>: add checks for integer overflow #2169

merged 18 commits into from
Aug 27, 2022

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Aug 29, 2021

Fixes #2168

@fsb4000 fsb4000 requested a review from a team as a code owner August 29, 2021 14:45
@fsb4000 fsb4000 marked this pull request as draft August 29, 2021 15:47
@fsb4000 @AlexGuteniev
address review comments
bc2c6b8 
Co-authored-by: Alex Guteniev <gutenev@gmail.com>
@fsb4000 fsb4000 marked this pull request as ready for review August 29, 2021 17:09
miscco
miscco reviewed Aug 29, 2021
View reviewed changes
stl/inc/regex Outdated Show resolved Hide resolved
@fsb4000 fsb4000 marked this pull request as draft August 30, 2021 15:31
@StephanTLavavej StephanTLavavej added the bug Something isn't working label Aug 30, 2021
@CaseyCarter CaseyCarter self-assigned this Sep 1, 2021
@fsb4000 fsb4000 marked this pull request as ready for review September 4, 2021 02:27
strega-nil-ms
strega-nil-ms reviewed Aug 10, 2022
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Going to push test changes, then check product code.

tests/std/tests/GH_002168_regex_overflow/test.cpp Outdated Show resolved Hide resolved
strega-nil-ms
strega-nil-ms suggested changes Aug 10, 2022
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Needs some more testing changes

stl/inc/regex Show resolved Hide resolved
stl/inc/regex Show resolved Hide resolved
stl/inc/regex Show resolved Hide resolved
@StephanTLavavej StephanTLavavej mentioned this pull request Aug 13, 2022
Closed
strega-nil-ms
strega-nil-ms approved these changes Aug 24, 2022
View reviewed changes
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

Thanks so much!

@strega-nil-ms strega-nil-ms removed their assignment Aug 24, 2022
StephanTLavavej
StephanTLavavej reviewed Aug 26, 2022
View reviewed changes
tests/std/tests/GH_002168_regex_overflow/test.cpp Outdated Show resolved Hide resolved
stl/inc/regex Outdated Show resolved Hide resolved
stl/inc/regex Outdated Show resolved Hide resolved
stl/inc/regex Outdated Show resolved Hide resolved
tests/std/tests/GH_002168_regex_overflow/test.cpp Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

Thanks, this looks correct and complete to me! FYI @strega-nil-ms I pushed minor style changes and additional test cases after you approved.

@StephanTLavavej StephanTLavavej self-assigned this Aug 26, 2022
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej changed the title <regex>: add check for overflow <regex>: add checks for integer overflow Aug 27, 2022
@StephanTLavavej StephanTLavavej merged commit f241c79 into microsoft:main Aug 27, 2022
@StephanTLavavej
Copy link
Member

Thanks two billion, one hundred forty-seven million, four hundred eighty-three thousand, six hundred forty-eight times for fixing this long-standing incorrect behavior! 🧮 🐞 😹

@fsb4000 fsb4000 deleted the fix2168 branch August 27, 2022 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexGuteniev AlexGuteniev AlexGuteniev approved these changes

@miscco miscco miscco left review comments

@strega-nil-ms strega-nil-ms strega-nil-ms approved these changes

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

<regex>: integer overflow on large backreference value
7 participants
@fsb4000 @StephanTLavavej @miscco @AlexGuteniev @strega-nil-ms @CaseyCarter @strega-nil