Skip to content

model/labels: fix regex with capture, wildcards, literal #17828

Merged
bboreham merged 2 commits intoprometheus:mainfrom
56quarters:56quarters/fast-regex-capture
Jan 12, 2026
Merged

model/labels: fix regex with capture, wildcards, literal #17828
bboreham merged 2 commits intoprometheus:mainfrom
56quarters:56quarters/fast-regex-capture

Conversation

@56quarters
Copy link
Contributor

@56quarters 56quarters commented Jan 9, 2026 •
edited
Loading

This change fixes an issue introduced in #17707. When a regex with a wildcard, literal, and final wildcard surounded by a capture group was parsed - the capture group was not removed first preventing optimizeConcatRegex from running.

Found via fuzz testing.

Which issue(s) does the PR fix:

Related #17707

Note to reviewers

The first commit contains a regex that will trigger the issue causing many tests to fail. The second commit contains the fix.

Does this PR introduce a user-facing change?

NONE

@56quarters
Copy link
Contributor Author

56quarters commented Jan 9, 2026

Fuzz testing output from https://github.com/grafana/mimir-prometheus/actions/runs/20844945972/job/59886370766?pr=1057

 --- FAIL: FuzzFastRegexMatcher_WithFuzzyRegularExpressions (27.10s)
    --- FAIL: FuzzFastRegexMatcher_WithFuzzyRegularExpressions (0.00s)
        regexp_test.go:867: 
            	Error Trace:	/home/runner/work/mimir-prometheus/mimir-prometheus/model/labels/regexp_test.go:867
            	            				/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.9.linux-amd64/src/reflect/value.go:584
            	            				/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.9.linux-amd64/src/reflect/value.go:368
            	            				/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.9.linux-amd64/src/testing/fuzz.go:340
            	            				/home/runner/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.9.linux-amd64/src/runtime/asm_amd64.s:1700
            	Error:      	Not equal: 
            	            	expected: false
            	            	actual  : true
            	Test:       	FuzzFastRegexMatcher_WithFuzzyRegularExpressions
            	Messages:   	regexp: ^(?s:(.*0.*))$ text: 1
    
    Failing input written to testdata/fuzz/FuzzFastRegexMatcher_WithFuzzyRegularExpressions/00991bf598e2f091
    To re-run:
    go test -run=FuzzFastRegexMatcher_WithFuzzyRegularExpressions/00991bf598e2f091

@56quarters
model/labels: fix regex with capture, wildcards, literal
31b5025 
This change fixes an issue introduced in prometheus#17707. When a regex
with a wildcard, literal, and final wildcard surounded by a
capture group was parsed - the capture group was not removed
first preventing `optimizeConcatRegex` from running.

Found via fuzz testing.

Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters
Clear capture groups
9e6338c 
Signed-off-by: Nick Pillitteri <nick.pillitteri@grafana.com>
@56quarters 56quarters force-pushed the 56quarters/fast-regex-capture branch from 80b20f3 to 9e6338c Compare January 9, 2026 20:30
@pr00se
Copy link
Contributor

pr00se commented Jan 9, 2026

This looks reasonable to me. Cursor suggests to do this before the regexp.Compile, but I don't think it matters functionally. I'll leave this to someone better versed in this code to approve. Thanks for taking a look!

@pr00se pr00se mentioned this pull request Jan 9, 2026
Closed
@56quarters 56quarters marked this pull request as ready for review January 11, 2026 18:13
charleskorn
charleskorn approved these changes Jan 11, 2026
View reviewed changes
Copy link
Contributor

@charleskorn charleskorn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for catching this and fixing it!

bboreham
bboreham approved these changes Jan 12, 2026
View reviewed changes
Copy link
Member

@bboreham bboreham left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'm ok to accept this, but I can't see how #17707 would cause it.
The previous code calls clearCapture on sub-expressions not on the top level.
Update: ok I see it now. If there is a capture then the intended optimisation doesn't work.

@bboreham
Copy link
Member

bboreham commented Jan 12, 2026

I'm going to squash this as the change is not huge and the commit message on the first commit seems inaccurate.

@bboreham bboreham enabled auto-merge (squash) January 12, 2026 15:43
@bboreham bboreham merged commit a769a7e into prometheus:main Jan 12, 2026
47 of 48 checks passed
@charleskorn charleskorn mentioned this pull request Jan 15, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bboreham bboreham bboreham approved these changes

+1 more reviewer

@charleskorn charleskorn charleskorn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@56quarters @pr00se @bboreham @charleskorn