Skip to content

Commit cd15f05

Browse files
committed
fix uniqueItems quadratic blowup
1 parent fb90377 commit cd15f05

2 files changed

Lines changed: 60 additions & 9 deletions

File tree

validator/array.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package validator
22

33
import (
44
"context"
5+
"encoding/json"
56
"fmt"
67
"reflect"
78

@@ -325,25 +326,42 @@ func (c *arrayValidator) evaluate(ctx context.Context, v any, st *evalState) (Re
325326
return nil, fmt.Errorf(`invalid value passed to ArrayValidator: array length %d exceeds maximum items %d`, length, *c.maxItems)
326327
}
327328

328-
// Check uniqueItems constraint
329-
if c.uniqueItems && length > 0 {
329+
// Check uniqueItems constraint.
330+
//
331+
// Rather than the naive O(n^2) pairwise comparison, bucket items by their
332+
// canonical JSON encoding and only compare within a bucket. reflect.DeepEqual
333+
// implies identical json.Marshal output, so the key never yields a false
334+
// negative (no missed duplicate); it may collide for DeepEqual-unequal values
335+
// (e.g. native int(1) vs float64(1.0), both encode as "1"), which is why a real
336+
// duplicate is still confirmed with reflect.DeepEqual to preserve existing
337+
// semantics. For JSON-decoded data (the untrusted path) a shared key implies
338+
// equality, so the first within-bucket comparison returns immediately and the
339+
// scan stays linear.
340+
if c.uniqueItems && acc.length > 1 {
341+
// json.Marshal never returns empty bytes for a valid value, so this
342+
// sentinel cannot collide with a real key. Items that fail to marshal
343+
// (exotic non-JSON values from reflection or a custom ArrayIndexResolver)
344+
// land here and are compared among themselves.
345+
const unmarshalableKey = "\x00unmarshalable"
346+
seen := make(map[string][]any, acc.length)
330347
for i := range acc.length {
331348
if err := ctx.Err(); err != nil {
332349
return nil, err
333350
}
334-
item1, err := acc.at(i)
351+
item, err := acc.at(i)
335352
if err != nil {
336353
return nil, fmt.Errorf(`invalid value passed to ArrayValidator: failed to resolve item %d: %w`, i, err)
337354
}
338-
for j := i + 1; j < acc.length; j++ {
339-
item2, err := acc.at(j)
340-
if err != nil {
341-
return nil, fmt.Errorf(`invalid value passed to ArrayValidator: failed to resolve item %d: %w`, j, err)
342-
}
343-
if reflect.DeepEqual(item1, item2) {
355+
key := unmarshalableKey
356+
if b, err := json.Marshal(item); err == nil {
357+
key = string(b)
358+
}
359+
for _, prev := range seen[key] {
360+
if reflect.DeepEqual(prev, item) {
344361
return nil, fmt.Errorf(`invalid value passed to ArrayValidator: duplicate items found, uniqueItems violation`)
345362
}
346363
}
364+
seen[key] = append(seen[key], item)
347365
}
348366
}
349367

validator/array_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package validator_test
22

33
import (
44
"context"
5+
"fmt"
56
"testing"
67

78
schema "github.com/lestrrat-go/json-schema"
@@ -712,3 +713,35 @@ func TestArrayValidatorComprehensive(t *testing.T) {
712713
}
713714
})
714715
}
716+
717+
// BenchmarkUniqueItems measures uniqueItems validation across array sizes. The
718+
// worst case for a naive O(n^2) implementation is an all-unique array (no early
719+
// termination), so that is what we feed it. Elements are float64 to mirror
720+
// JSON-decoded numbers.
721+
func BenchmarkUniqueItems(b *testing.B) {
722+
for _, n := range []int{100, 1000, 5000} {
723+
b.Run(fmt.Sprintf("n=%d", n), func(b *testing.B) {
724+
s, err := schema.NewBuilder().
725+
Types(schema.ArrayType).
726+
UniqueItems(true).
727+
Build()
728+
require.NoError(b, err)
729+
730+
v, err := validator.Compile(b.Context(), s)
731+
require.NoError(b, err)
732+
733+
data := make([]any, n)
734+
for i := range data {
735+
data[i] = float64(i)
736+
}
737+
738+
ctx := b.Context()
739+
b.ResetTimer()
740+
for range b.N {
741+
if _, err := v.Validate(ctx, data); err != nil {
742+
b.Fatal(err)
743+
}
744+
}
745+
})
746+
}
747+
}

0 commit comments

Comments
 (0)