Skip to content

Commit 20113a6

Browse files
ajgSilvioclaude
authored
Add cycle detection to the encoder (#33)
* Move Future Work items from README to TODO.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add cycle detection to the encoder Thread a seen-pointer set through encoding functions so that self-referential pointers and maps are detected and returned as errors instead of causing a stack overflow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove Limitations section from README Circular values are now detected and reported as errors, so the limitation no longer applies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Silvio <silvio@MBA13.home> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e5aae99 commit 20113a6

File tree

4 files changed

+76
-34
lines changed

4 files changed

+76
-34
lines changed

README.md

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -240,23 +240,6 @@ Custom: foo.bar%2Fqux=XYZ
240240

241241
(`%5C` and `%2F` represent `\` and `/`, respectively.)
242242

243-
Limitations
244-
-----------
245-
246-
- Circular (self-referential) values are untested.
247-
248-
Future Work
249-
-----------
250-
251-
The following items would be nice to have in the future—though they are not being worked on yet:
252-
253-
- An option to automatically treat all field names in `camelCase` or `underscore_case`.
254-
- Built-in support for the types in [`math/big`](http://golang.org/pkg/math/big/).
255-
- Built-in support for the types in [`image/color`](http://golang.org/pkg/image/color/).
256-
- Improve encoding/decoding by reading/writing directly from/to the `io.Reader`/`io.Writer` when possible, rather than going through an intermediate representation (i.e. `node`) which requires more memory.
257-
258-
(Feel free to implement any of these and then send a pull request.)
259-
260243
Related Work
261244
------------
262245

TODO.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,7 @@ TODO
22
====
33

44
- Document IgnoreCase and IgnoreUnknownKeys in README.
5+
- An option to automatically treat all field names in `camelCase` or `underscore_case`.
6+
- Built-in support for the types in [`math/big`](http://golang.org/pkg/math/big/).
7+
- Built-in support for the types in [`image/color`](http://golang.org/pkg/image/color/).
8+
- Improve encoding/decoding by reading/writing directly from/to the `io.Reader`/`io.Writer` when possible, rather than going through an intermediate representation (i.e. `node`) which requires more memory.

encode.go

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,11 @@ func encodeToNode(v reflect.Value, z bool, o bool) (n node, err error) {
118118
err = fmt.Errorf("%v", e)
119119
}
120120
}()
121-
return getNode(encodeValue(v, z, o)), nil
121+
seen := make(map[uintptr]bool)
122+
return getNode(encodeValue(v, z, o, seen)), nil
122123
}
123124

124-
func encodeValue(v reflect.Value, z bool, o bool) interface{} {
125+
func encodeValue(v reflect.Value, z bool, o bool, seen map[uintptr]bool) interface{} {
125126
t := v.Type()
126127
k := v.Kind()
127128

@@ -132,21 +133,35 @@ func encodeValue(v reflect.Value, z bool, o bool) interface{} {
132133
}
133134

134135
switch k {
135-
case reflect.Ptr, reflect.Interface:
136-
return encodeValue(v.Elem(), z, o)
136+
case reflect.Ptr:
137+
ptr := v.Pointer()
138+
if seen[ptr] {
139+
panic("form: encoding a cycle via " + t.String())
140+
}
141+
seen[ptr] = true
142+
defer delete(seen, ptr)
143+
return encodeValue(v.Elem(), z, o, seen)
144+
case reflect.Interface:
145+
return encodeValue(v.Elem(), z, o, seen)
137146
case reflect.Struct:
138147
if t.ConvertibleTo(timeType) {
139148
return encodeTime(v)
140149
} else if t.ConvertibleTo(urlType) {
141150
return encodeURL(v)
142151
}
143-
return encodeStruct(v, z, o)
152+
return encodeStruct(v, z, o, seen)
144153
case reflect.Slice:
145-
return encodeSlice(v, z, o)
154+
return encodeSlice(v, z, o, seen)
146155
case reflect.Array:
147-
return encodeArray(v, z, o)
156+
return encodeArray(v, z, o, seen)
148157
case reflect.Map:
149-
return encodeMap(v, z, o)
158+
ptr := v.Pointer()
159+
if seen[ptr] {
160+
panic("form: encoding a cycle via " + t.String())
161+
}
162+
seen[ptr] = true
163+
defer delete(seen, ptr)
164+
return encodeMap(v, z, o, seen)
150165
case reflect.Invalid, reflect.Uintptr, reflect.UnsafePointer, reflect.Chan, reflect.Func:
151166
panic(t.String() + " has unsupported kind " + t.Kind().String())
152167
default:
@@ -160,7 +175,7 @@ type encoderField struct {
160175
omitempty bool
161176
}
162177

163-
func encodeStruct(v reflect.Value, z bool, o bool) interface{} {
178+
func encodeStruct(v reflect.Value, z bool, o bool, seen map[uintptr]bool) interface{} {
164179
fields := collectFields(v.Type())
165180
n := node{}
166181
for _, f := range fields {
@@ -171,7 +186,7 @@ func encodeStruct(v reflect.Value, z bool, o bool) interface{} {
171186
if (o || f.omitempty) && isEmptyValue(fv) {
172187
continue
173188
}
174-
n[f.name] = encodeValue(fv, z, o)
189+
n[f.name] = encodeValue(fv, z, o, seen)
175190
}
176191
return n
177192
}
@@ -326,31 +341,31 @@ func isLeafStruct(ft reflect.Type) bool {
326341
return ft.Implements(textMarshalerType) || reflect.PtrTo(ft).Implements(textMarshalerType)
327342
}
328343

329-
func encodeMap(v reflect.Value, z bool, o bool) interface{} {
344+
func encodeMap(v reflect.Value, z bool, o bool, seen map[uintptr]bool) interface{} {
330345
n := node{}
331346
for _, i := range v.MapKeys() {
332-
k := getString(encodeValue(i, z, o))
333-
n[k] = encodeValue(v.MapIndex(i), z, o)
347+
k := getString(encodeValue(i, z, o, seen))
348+
n[k] = encodeValue(v.MapIndex(i), z, o, seen)
334349
}
335350
return n
336351
}
337352

338-
func encodeArray(v reflect.Value, z bool, o bool) interface{} {
353+
func encodeArray(v reflect.Value, z bool, o bool, seen map[uintptr]bool) interface{} {
339354
n := node{}
340355
for i := 0; i < v.Len(); i++ {
341-
n[strconv.Itoa(i)] = encodeValue(v.Index(i), z, o)
356+
n[strconv.Itoa(i)] = encodeValue(v.Index(i), z, o, seen)
342357
}
343358
return n
344359
}
345360

346-
func encodeSlice(v reflect.Value, z bool, o bool) interface{} {
361+
func encodeSlice(v reflect.Value, z bool, o bool, seen map[uintptr]bool) interface{} {
347362
t := v.Type()
348363
if t.Elem().Kind() == reflect.Uint8 {
349364
return string(v.Bytes()) // Encode byte slices as a single string by default.
350365
}
351366
n := node{}
352367
for i := 0; i < v.Len(); i++ {
353-
n[strconv.Itoa(i)] = encodeValue(v.Index(i), z, o)
368+
n[strconv.Itoa(i)] = encodeValue(v.Index(i), z, o, seen)
354369
}
355370
return n
356371
}

encode_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,46 @@ func TestEncode_OmitEmpty(t *testing.T) {
137137
}
138138
}
139139

140+
func TestEncode_Cycle(t *testing.T) {
141+
t.Run("self-referential struct pointer", func(t *testing.T) {
142+
type Cyclic struct {
143+
Name string
144+
Next *Cyclic
145+
}
146+
a := &Cyclic{Name: "a"}
147+
a.Next = a
148+
149+
if _, err := EncodeToString(a); err == nil {
150+
t.Error("expected error for cyclic struct pointer, got nil")
151+
}
152+
})
153+
154+
t.Run("map containing itself", func(t *testing.T) {
155+
m := map[string]interface{}{}
156+
m["self"] = m
157+
158+
if _, err := EncodeToString(m); err == nil {
159+
t.Error("expected error for cyclic map, got nil")
160+
}
161+
})
162+
163+
t.Run("non-cyclic pointer sharing (DAG)", func(t *testing.T) {
164+
type Node struct {
165+
Value string
166+
}
167+
type DAG struct {
168+
A *Node
169+
B *Node
170+
}
171+
shared := &Node{Value: "shared"}
172+
dag := DAG{A: shared, B: shared}
173+
174+
if _, err := EncodeToString(dag); err != nil {
175+
t.Errorf("unexpected error for DAG: %s", err)
176+
}
177+
})
178+
}
179+
140180
func TestEncode_ConflictResolution(t *testing.T) {
141181
for _, c := range []struct {
142182
name string

0 commit comments

Comments
 (0)