Skip to content

fix: handle missing/null targeting keys in fractional evaluator #1949

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

Merged
toddbaert merged 2 commits into from
Apr 20, 2026
+72 −0
Merged

fix: handle missing/null targeting keys in fractional evaluator #1949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9ff5a3
fix: handle missing/null targeting keys in fractional evaluator
utafrali Apr 16, 2026
2f03138
Address review feedback
utafrali Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions core/pkg/evaluator/fractional.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ func (fe *Fractional) Evaluate(values, data any) any {
return nil
}

if feDistributions == nil {
Copy link
Copy Markdown
Member

@leakonvalinka leakonvalinka Apr 17, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Can this ever be the case?
Edit: Oh yes, I see, with your changes below.

return nil
}

hashValue := uint32(murmur3.StringSum32(valueToDistribute))
return distributeValue(hashValue, feDistributions)
}
Expand Down Expand Up @@ -78,11 +82,18 @@ if len(valuesArray) < 1 {
valuesArray = valuesArray[1:]
}

if dataMap[targetingKeyKey] == nil {
return "", nil, nil
}
targetingKey, ok := dataMap[targetingKeyKey].(string)
if !ok {
Copy link
Copy Markdown
Member

@leakonvalinka leakonvalinka Apr 17, 2026

Choose a reason for hiding this comment

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

Just a suggestion: You could also simply add a check here for an empty string.

return "", nil, fmt.Errorf("flag %q: bucketing value not supplied and no targetingKey in context", flagKey)
}

if targetingKey == "" {
return "", nil, nil
}

bucketBy = fmt.Sprintf("%s%s", properties.FlagKey, targetingKey)
}

Expand Down
61 changes: 61 additions & 0 deletions core/pkg/evaluator/fractional_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,67 @@ func TestFractionalEvaluation(t *testing.T) {
expectedValue: blueHex,
expectedReason: model.TargetingMatchReason,
},
"null targetingKey returns default variant": {
flags: []model.Flag{{
Key: "headerColor",
State: "ENABLED",
DefaultVariant: redVariant,
Variants: colorVariants,
Targeting: []byte(`{
"fractional": [
["blue", 50],
["green", 50]
]
}`),
}},
flagKey: "headerColor",
context: map[string]any{
"targetingKey": nil,
},
expectedVariant: redVariant,
expectedValue: redHex,
expectedReason: model.DefaultReason,
},
"missing targetingKey returns default variant": {
flags: []model.Flag{{
Key: "headerColor",
State: "ENABLED",
DefaultVariant: redVariant,
Variants: colorVariants,
Targeting: []byte(`{
"fractional": [
["blue", 50],
["green", 50]
]
}`),
}},
flagKey: "headerColor",
context: map[string]any{},
expectedVariant: redVariant,
expectedValue: redHex,
expectedReason: model.DefaultReason,
},
"empty targetingKey returns default variant": {
flags: []model.Flag{{
Key: "headerColor",
State: "ENABLED",
DefaultVariant: redVariant,
Variants: colorVariants,
Targeting: []byte(`{
"fractional": [
["blue", 50],
["green", 50]
]
}`),
}},
flagKey: "headerColor",
context: map[string]any{
"targetingKey": "",
Copy link
Copy Markdown
Member

@leakonvalinka leakonvalinka Apr 16, 2026

Choose a reason for hiding this comment

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

Hi, this wasn't defined in the issue yet when you opened your PR, but the suggested fix is that targetingKey = "" should be treated the same as targetingkey = nil and a non-existent targeting key and therefore result in falling back to the default. :)

Copy link
Copy Markdown
Member

@toddbaert toddbaert Apr 16, 2026

Choose a reason for hiding this comment

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

+1, see the testbed test for this:
https://github.com/open-feature/flagd-testbed/blob/484ada4dd979f663214c024f3cb601141328a1b5/gherkin/targeting.feature#L376-L381

},
expectedVariant: redVariant,
expectedValue: redHex,
expectedReason: model.DefaultReason,
},
"single-entry always returns the sole variant": {
flags: []model.Flag{{
Key: "headerColor",
Expand Down
Loading