Skip to content

Commit 1e1aac0

Browse files
authored
Unique args: Guard against recursive types (#1090)
Also reported over in #1087, the unique module will overflow if encountering a recursive type as it looks for unique keys to consider. Here, try to be a little smarter about this, but not overly so by only processing each type once. This means that inner recursive types always just get treatment as a fully encoded JSON blob, but that's about as good of a way to handle them as I can think of anyway. Take this example: type RecursiveType struct { NotUnique string `river:"-"` Recursive *RecursiveType `river:"unique"` // inner recursive type ignored by River String string `river:"unique"` } type TaskJobArgs struct { JobArgsStaticKind Recursive RecursiveType `river:"unique"` } return TaskJobArgs{ JobArgsStaticKind: JobArgsStaticKind{kind: "worker_7"}, Recursive: RecursiveType{ Recursive: &RecursiveType{ String: "level2", }, String: "level1", }, } Generated output is: &kind=worker_7&args={"Recursive":{"Recursive":{"NotUnique":"","Recursive":null,"String":"level2"},"String":"level1"}} Notably, we see "NotUnique" show up in the inner recursive type because River has given up processing that and just treated the entire inner "Recursive" as a JSON value. The outer type does not have "NotUnique" because River saw the `river:"-"` annotation and knew not to treat it as unique.
1 parent 5b1fa35 commit 1e1aac0

3 files changed

Lines changed: 48 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Fixed
1111

1212
- Unique args: Handle embedded fields that are not structs. [PR #1088](https://github.com/riverqueue/river/pull/1088).
13+
- Fix stack overflow when handling `river:"unique"` annotations on recursive types. [PR #1090](https://github.com/riverqueue/river/pull/1090).
1314

1415
## [0.27.0] - 2025-11-14
1516

internal/dbunique/db_unique_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,39 @@ func TestUniqueKey(t *testing.T) {
360360
// args JSON should be sorted alphabetically:
361361
expectedJSON: `&kind=worker_1&args={}`,
362362
},
363+
{
364+
name: "ByArgsRecursiveType",
365+
argsFunc: func() rivertype.JobArgs {
366+
type RecursiveType struct {
367+
NotUnique string `river:"-"`
368+
Recursive *RecursiveType `river:"unique"` // inner recursive type ignored by River
369+
String string `river:"unique"`
370+
}
371+
type TaskJobArgs struct {
372+
JobArgsStaticKind
373+
374+
Recursive RecursiveType `river:"unique"`
375+
}
376+
return TaskJobArgs{
377+
JobArgsStaticKind: JobArgsStaticKind{kind: "worker_7"},
378+
Recursive: RecursiveType{
379+
Recursive: &RecursiveType{
380+
String: "level2",
381+
},
382+
String: "level1",
383+
},
384+
}
385+
},
386+
uniqueOpts: UniqueOpts{ByArgs: true},
387+
388+
// Notably, "NotUnique" shows up here inside the inner recursive
389+
// type because when River saw the recursive typed markd with
390+
// `unique` again, it just gave up and returned the entire the
391+
// entire key which is then extracted by gjson. The top-level type
392+
// also has a "NotUnique", but that's not returned because River was
393+
// processing this type for the first time.
394+
expectedJSON: `&kind=worker_7&args={"Recursive":{"Recursive":{"NotUnique":"","Recursive":null,"String":"level2"},"String":"level1"}}`,
395+
},
363396
{
364397
name: "CustomByStateWithPeriod",
365398
argsFunc: func() rivertype.JobArgs {

internal/dbunique/unique_fields.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ func extractUniqueValues(encodedArgs []byte, uniqueKeys []string) []string {
4242
// getSortedUniqueFields uses reflection to retrieve the JSON keys of fields
4343
// marked with `river:"unique"` among potentially other comma-separated values.
4444
// The return values are the JSON keys using the same logic as the `json` struct tag.
45-
func getSortedUniqueFields(typ reflect.Type, path []string) ([]string, error) {
45+
//
46+
// typesSeen should be a map passed through to make sure that recursive types
47+
// don't cause a stack overflow.
48+
func getSortedUniqueFields(typ reflect.Type, path []string, typesSeen map[reflect.Type]struct{}) ([]string, error) {
4649
// Handle pointer to struct
4750
if typ.Kind() == reflect.Ptr {
4851
typ = typ.Elem()
@@ -53,6 +56,14 @@ func getSortedUniqueFields(typ reflect.Type, path []string) ([]string, error) {
5356
return nil, fmt.Errorf("expected struct, got %T", typ.Name())
5457
}
5558

59+
// Stop when encountering a recursive type. This has the effect of the
60+
// entire subfield's value being extracted by gjson, but this is about as
61+
// right of a way to handle it as any other I can think of.
62+
if _, ok := typesSeen[typ]; ok {
63+
return nil, nil
64+
}
65+
typesSeen[typ] = struct{}{}
66+
5667
var uniqueFields []string
5768

5869
// Iterate over all fields
@@ -97,7 +108,7 @@ func getSortedUniqueFields(typ reflect.Type, path []string) ([]string, error) {
97108
fullPath = append(path, uniqueName) //nolint:gocritic
98109
}
99110

100-
uniqueSubFields, err := getSortedUniqueFields(field.Type, fullPath)
111+
uniqueSubFields, err := getSortedUniqueFields(field.Type, fullPath, typesSeen)
101112
if err != nil {
102113
return nil, err
103114
}
@@ -140,7 +151,7 @@ func getSortedUniqueFieldsCached(args rivertype.JobArgs) ([]string, error) {
140151
cacheMutex.RUnlock()
141152

142153
// Not in cache; retrieve using reflection
143-
fields, err := getSortedUniqueFields(reflect.TypeOf(args), nil)
154+
fields, err := getSortedUniqueFields(reflect.TypeOf(args), nil, make(map[reflect.Type]struct{}))
144155
if err != nil {
145156
return nil, err
146157
}

0 commit comments

Comments
 (0)