Conversation
…icking matchRegexp type-asserts rx to *regexp.Regexp before use, so a typed nil passes the assertion and MatchString then dereferences it. Both Regexp and NotRegexp panic where a result was expected, in the caller's test run, with the trace pointing into this package. A typed nil is a *regexp.Regexp that was declared and never assigned. There is no pattern behind it, so nothing can match: report false, and let NotRegexp invert it as usual.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RegexpandNotRegexppanic on a typed nil*regexp.Regexpinstead of reporting a result.Changes
matchRegexp(used byRegexpandNotRegexp): a typed nil*regexp.Regexpreports no match instead of being dereferenced.RegexpandNotRegexp.Split out of #1971 at dolmen's request — the two causes are unrelated. That PR keeps the nil
conditionand non-positivetickguards, which are about arguments that reachtime.NewTickerand the goroutine this package spawns.Motivation
matchRegexptype-assertsrxto*regexp.Regexpbefore using it. A typed nil passes the assertion — it has the right type, and a nil value — soMatchStringthen dereferences it. The panic lands in the caller's test run with the trace pointing into this package rather than at the call, and there is no way for the caller to recover from it:A typed nil here is a
*regexp.Regexpthat was declared and never assigned. There is no pattern behind it, so nothing can match —false, whichNotRegexpinverts as usual. The alternative, treating it as an error, would change what callers ofRegexpsee for a value that is not a usable pattern either way, and would be a larger change than the bug needs.Related issues
Split from #1971. No separate issue — happy to open one if you prefer the issue-first route.