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: implement nil check for CEL program in matchRoutingRule #6280

Merged

Conversation

dmutterSF
Copy link
Contributor

Avoid nil pointer dereference when evaluating expression.
Resolves #6036

Haven't written go before so this might not be the proper or most optimal solution

@dmutterSF dmutterSF requested review from a team as code owners April 24, 2023 14:03
@yaron2
Copy link
Member

yaron2 commented Apr 24, 2023

program is an unexported (private) field of Expr, so the place to do the check is here:

out, _, err := e.program.Eval(variables)
.

@dmutterSF dmutterSF force-pushed the subscription-rule-match-program-nil-check branch 3 times, most recently from 6d107b4 to 1ffab39 Compare April 25, 2023 15:33
Signed-off-by: dmutter <dmutter@starface.com>
Signed-off-by: dmutter <dmutter@starface.com>
@dmutterSF dmutterSF force-pushed the subscription-rule-match-program-nil-check branch from 1ffab39 to d8b3eb8 Compare April 26, 2023 07:33
@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

Merging #6280 (ac0bae5) into master (3b0defb) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #6280      +/-   ##
==========================================
- Coverage   64.40%   64.39%   -0.01%     
==========================================
  Files         189      189              
  Lines       18881    18885       +4     
==========================================
+ Hits        12160    12161       +1     
- Misses       5753     5759       +6     
+ Partials      968      965       -3     
Impacted Files Coverage Δ
pkg/expr/expr.go 81.39% <100.00%> (+7.03%) ⬆️

... and 2 files with indirect coverage changes

Signed-off-by: yaron2 <schneider.yaron@live.com>
@yaron2 yaron2 merged commit 1379bee into dapr:master May 3, 2023
27 of 29 checks passed
@yaron2
Copy link
Member

yaron2 commented May 3, 2023

Thanks for your contribution @dmutterSF

@yaron2 yaron2 assigned dmutterSF and unassigned yaron2 May 3, 2023
@yaron2 yaron2 added this to the v1.11 milestone May 3, 2023
@dmutterSF dmutterSF deleted the subscription-rule-match-program-nil-check branch May 3, 2023 14:55
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.

Panic when subscriber listen message in Redis
4 participants