From 396274445068c51311d2d4f2aaf6b04fe874e649 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 6 Jul 2021 21:41:18 -0700 Subject: [PATCH] util/deephash: prevent infinite loop on map cycle Fixes #2340 Signed-off-by: Brad Fitzpatrick --- util/deephash/deephash.go | 22 ++++++++++++++++++++++ util/deephash/deephash_test.go | 12 ++++++++++++ 2 files changed, 34 insertions(+) diff --git a/util/deephash/deephash.go b/util/deephash/deephash.go index 8a1e65f85..38b6eddb4 100644 --- a/util/deephash/deephash.go +++ b/util/deephash/deephash.go @@ -169,6 +169,28 @@ func (h *hasher) print(v reflect.Value) (acyclic bool) { case reflect.Interface: return h.print(v.Elem()) case reflect.Map: + // TODO(bradfitz): ideally we'd avoid these map + // operations to detect cycles if we knew from the map + // element type that there no way to form a cycle, + // which is the common case. Notably, we don't care + // about hashing the same map+contents twice in + // different parts of the tree. In fact, we should + // ideally. (And this prevents it) We should only stop + // hashing when there's a cycle. What we should + // probably do is make sure we enumerate the data + // structure tree is a fixed order and then give each + // pointer an increasing number, and when we hit a + // dup, rather than emitting nothing, we should emit a + // "value #12" reference. Which implies that all things + // emit to the bufio.Writer should be type-tagged so + // we can distinguish loop references without risk of + // collisions. + ptr := v.Pointer() + if visited[ptr] { + return false + } + visited[ptr] = true + if h.hashMapAcyclic(v) { return true } diff --git a/util/deephash/deephash_test.go b/util/deephash/deephash_test.go index dc8a92453..e433674c8 100644 --- a/util/deephash/deephash_test.go +++ b/util/deephash/deephash_test.go @@ -226,3 +226,15 @@ func TestSHA256EqualHex(t *testing.T) { } } } + +// verify this doesn't loop forever, as it used to (Issue 2340) +func TestMapCyclicFallback(t *testing.T) { + type T struct { + M map[string]interface{} + } + v := &T{ + M: map[string]interface{}{}, + } + v.M["m"] = v.M + Hash(v) +}