diff --git a/pkg/rules/number_coerce_test.go b/pkg/rules/number_coerce_test.go index e33dfa8..1ff24be 100644 --- a/pkg/rules/number_coerce_test.go +++ b/pkg/rules/number_coerce_test.go @@ -2,6 +2,7 @@ package rules_test import ( "fmt" + "math" "strings" "testing" @@ -316,6 +317,22 @@ func TestCoerceFloat64ToFloat32_CommonDecimals(t *testing.T) { testhelpers.MustApplyMutation(t, ruleSet, float64(0.3), float32(0.3)) } +// TestTryCoerceFloatToFloat_NaNAndInf tests tryCoerceFloatToFloat coverage: +// - float64 NaN/Inf -> float32 returns CodeRange (cannot represent in float32) +// - float32 NaN -> float64 returns CodeRange (round-trip fails: NaN != NaN) +// Same-type float (e.g. float32->Float32) takes coerceFloat's case T path and does not call tryCoerceFloatToFloat. +func TestTryCoerceFloatToFloat_NaNAndInf(t *testing.T) { + // float64 -> float32: NaN and Inf are rejected (tryCoerceFloatToFloat NaN/Inf branch) + ruleSet32 := rules.Float32().Any() + testhelpers.MustNotApply(t, ruleSet32, float64(math.NaN()), errors.CodeRange) + testhelpers.MustNotApply(t, ruleSet32, float64(math.Inf(1)), errors.CodeRange) + testhelpers.MustNotApply(t, ruleSet32, float64(math.Inf(-1)), errors.CodeRange) + + // float32 NaN -> float64: round-trip fails (NaN != NaN) (tryCoerceFloatToFloat round-trip branch) + ruleSet64 := rules.Float64().Any() + testhelpers.MustNotApply(t, ruleSet64, float32(math.NaN()), errors.CodeRange) +} + // TestFloat32EqualityCheckFailure tests: // - Returns error when float32 cannot exactly represent the integer value func TestFloat32EqualityCheckFailure(t *testing.T) { diff --git a/pkg/rules/object.go b/pkg/rules/object.go index 3c2a23d..e919eba 100644 --- a/pkg/rules/object.go +++ b/pkg/rules/object.go @@ -551,8 +551,23 @@ func (v *ObjectRuleSet[T, TK, TV]) evaluateKeyRules(ctx context.Context, out *T, allErrors := errors.Collection() var emptyKey TK + // Pre caching a list of dynamic buckets lets us avoid extra loops. + // This method is faster in all cases where there is at least one bucket and the input has dynamic values. + // We build this before knownKeys so we can track keys when dynamic buckets exist: otherwise with + // WithUnknown() and struct output, knownKeys would not track and every key would be treated as + // "unknown", causing SetBucket to be called with raw input instead of validated output (panic when + // bucket value type differs from input, e.g. Interface.WithCast). + dynamicBuckets := make([]*ObjectRuleSet[T, TK, TV], 0) + for currentRuleSet := v; currentRuleSet != nil; currentRuleSet = currentRuleSet.parent { + if currentRuleSet.bucket != emptyKey { + dynamicBuckets = append(dynamicBuckets, currentRuleSet) + } + } + // Tracks which keys are known so we can create errors for unknown keys. - knownKeys := newKnownKeys[TK]((!v.allowUnknown || s.Map()) && fromMap) + // When dynamic buckets exist we must track so keys processed by evaluateKeyRule are not + // overwritten in the "unknown keys" path with raw input. + knownKeys := newKnownKeys[TK](((!v.allowUnknown || s.Map()) && fromMap) || len(dynamicBuckets) > 0) // Add each key to the counter. // We need this because conditional keys cannot run until all rule sets are run since rule sets are able @@ -581,15 +596,6 @@ func (v *ObjectRuleSet[T, TK, TV]) evaluateKeyRules(ctx context.Context, out *T, defer close(errorsCh) var outValueMutex sync.Mutex - // Pre caching a list of dynamic buckets lets us avoid extra loops. - // This method is faster in all cases where there is at least one bucket and the input has dynamic values - dynamicBuckets := make([]*ObjectRuleSet[T, TK, TV], 0) - for currentRuleSet := v; currentRuleSet != nil; currentRuleSet = currentRuleSet.parent { - if currentRuleSet.bucket != emptyKey { - dynamicBuckets = append(dynamicBuckets, currentRuleSet) - } - } - // Wait for all the rules to finish var wg sync.WaitGroup diff --git a/pkg/rules/object_test.go b/pkg/rules/object_test.go index 0dc3464..0554893 100644 --- a/pkg/rules/object_test.go +++ b/pkg/rules/object_test.go @@ -1711,6 +1711,96 @@ func TestWithDynamicBucketToStruct(t *testing.T) { } } +// valueListForBucketTest is an interface used to reproduce the bug where WithDynamicBucket + +// WithDynamicKey(Interface.WithCast) + WithUnknown + struct output caused SetBucket to receive +// raw input ([]string) instead of validated output, leading to a reflect SetMapIndex panic. +type valueListForBucketTest interface { + Values() []string + doNotExtend() +} + +type fieldListMapForTest map[string]bool + +func (fl fieldListMapForTest) Values() []string { + keys := make([]string, 0, len(fl)) + for k := range fl { + keys = append(keys, k) + } + return keys +} + +func (fieldListMapForTest) doNotExtend() {} + +func newFieldListForTest(fields ...string) valueListForBucketTest { + out := make(fieldListMapForTest, len(fields)) + for _, f := range fields { + out[f] = true + } + return out +} + +// TestWithDynamicBucketAndDynamicKeyInterfaceToStruct reproduces a bug where using WithDynamicKey +// with rules.Interface[T].WithCast(...) and WithDynamicBucket to a struct field map, plus +// WithUnknown(), caused the "unknown keys" path to call SetBucket with raw input (e.g. []string) +// instead of the validated output (e.g. ValueList), triggering: +// panic: reflect.Value.SetMapIndex: value of type []string is not assignable to type T +// The fix is to track known keys when dynamic buckets exist so keys already handled by +// evaluateKeyRule are not re-processed in the unknown path. +func TestWithDynamicBucketAndDynamicKeyInterfaceToStruct(t *testing.T) { + type queryData struct { + Fields map[string]valueListForBucketTest + Filters map[string][]string + } + + stringQueryValueRuleSet := rules.Slice[string]().WithItemRuleSet(rules.String()).WithMaxLen(1) + fieldKeyRule := rules.String().WithRegexp(regexp.MustCompile(`^fields\[[^\]]+\]$`), "") + filterKeyRule := rules.String().WithRegexp(regexp.MustCompile(`^filter\[[^\]]+\]$`), "") + + fieldsRuleSet := rules.Interface[valueListForBucketTest]().WithCast( + func(ctx context.Context, value any) (valueListForBucketTest, errors.ValidationErrorCollection) { + var strs []string + if errs := stringQueryValueRuleSet.Apply(ctx, value, &strs); errs != nil { + return nil, errs + } + if len(strs) == 0 { + return newFieldListForTest(), nil + } + return newFieldListForTest(stringsHelper.Split(strs[0], ",")...), nil + }, + ) + filterRuleSet := rules.Slice[string]().WithItemRuleSet(rules.String()) + + ruleSet := rules.Struct[queryData](). + WithDynamicKey(fieldKeyRule, fieldsRuleSet.Any()). + WithDynamicKey(filterKeyRule, filterRuleSet.Any()). + WithDynamicBucket(fieldKeyRule, "Fields"). + WithDynamicBucket(filterKeyRule, "Filters"). + WithUnknown() + + parsed, err := url.ParseQuery(`fields[articles]=abc,xyz`) + if err != nil { + t.Fatalf("ParseQuery: %v", err) + } + + var output queryData + errs := ruleSet.Apply(context.Background(), parsed, &output) + if errs != nil { + t.Fatalf("Apply: %v", errs) + } + if output.Fields == nil { + t.Fatal("output.Fields is nil") + } + vl, ok := output.Fields["fields[articles]"] + if !ok { + t.Fatal("fields[articles] not in output.Fields") + } + // If the bug were present we would have panicked in SetBucket ([]string not assignable to ValueList). + vals := vl.Values() + if len(vals) != 2 || (vals[0] != "abc" && vals[1] != "abc") || (vals[0] != "xyz" && vals[1] != "xyz") { + t.Errorf("expected Values() to contain abc and xyz, got %v", vals) + } +} + // TestWithConditionalDynamicBucket tests: // - If no dynamic bucket is matched then the key is considered unknown // - Dynamic buckets are not created unless condition is met