From 02057906f7272a4787b8a0b5b7cafff8ad3024f0 Mon Sep 17 00:00:00 2001 From: Kyle Nusbaum Date: Wed, 18 Mar 2020 19:17:50 +0000 Subject: [PATCH] context: prevent creation of invalid contexts This commit makes it impossible to create derived contexts with nil parents. Previously it was possible to create derived contexts with nil parents, and invalid contexts could propogate through the program. Eventually this can cause a panic downstream, which is difficult to trace back to the source of the error. Although `WithCancel` and `WithDeadline` already panic if `parent` is `nil`, this adds explicit checks to give a useful message in the panic. Fixes #37908 Change-Id: I70fd01f6539c1b0da0e775fc5457e32e7075e52c GitHub-Last-Rev: 1b7dadd7db9ba42952644ad5e9a49591d6a5191f GitHub-Pull-Request: golang/go#37898 Reviewed-on: https://go-review.googlesource.com/c/go/+/223777 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/context/context.go | 9 +++++++++ src/context/context_test.go | 15 +++++++++++++++ src/context/x_test.go | 1 + 3 files changed, 25 insertions(+) diff --git a/src/context/context.go b/src/context/context.go index b561968f31c764..b3fdb8277afc37 100644 --- a/src/context/context.go +++ b/src/context/context.go @@ -230,6 +230,9 @@ type CancelFunc func() // Canceling this context releases resources associated with it, so code should // call cancel as soon as the operations running in this Context complete. func WithCancel(parent Context) (ctx Context, cancel CancelFunc) { + if parent == nil { + panic("cannot create context from nil parent") + } c := newCancelCtx(parent) propagateCancel(parent, &c) return &c, func() { c.cancel(true, Canceled) } @@ -425,6 +428,9 @@ func (c *cancelCtx) cancel(removeFromParent bool, err error) { // Canceling this context releases resources associated with it, so code should // call cancel as soon as the operations running in this Context complete. func WithDeadline(parent Context, d time.Time) (Context, CancelFunc) { + if parent == nil { + panic("cannot create context from nil parent") + } if cur, ok := parent.Deadline(); ok && cur.Before(d) { // The current deadline is already sooner than the new one. return WithCancel(parent) @@ -511,6 +517,9 @@ func WithTimeout(parent Context, timeout time.Duration) (Context, CancelFunc) { // struct{}. Alternatively, exported context key variables' static // type should be a pointer or interface. func WithValue(parent Context, key, val interface{}) Context { + if parent == nil { + panic("cannot create context from nil parent") + } if key == nil { panic("nil key") } diff --git a/src/context/context_test.go b/src/context/context_test.go index da29ed0c2b903e..98c6683335370f 100644 --- a/src/context/context_test.go +++ b/src/context/context_test.go @@ -667,6 +667,21 @@ func XTestWithValueChecksKey(t testingT) { } } +func XTestInvalidDerivedFail(t testingT) { + panicVal := recoveredValue(func() { WithCancel(nil) }) + if panicVal == nil { + t.Error("expected panic") + } + panicVal = recoveredValue(func() { WithDeadline(nil, time.Now().Add(shortDuration)) }) + if panicVal == nil { + t.Error("expected panic") + } + panicVal = recoveredValue(func() { WithValue(nil, "foo", "bar") }) + if panicVal == nil { + t.Error("expected panic") + } +} + func recoveredValue(fn func()) (v interface{}) { defer func() { v = recover() }() fn() diff --git a/src/context/x_test.go b/src/context/x_test.go index e85ef2d50e5fd8..00eca72d5aff0a 100644 --- a/src/context/x_test.go +++ b/src/context/x_test.go @@ -26,5 +26,6 @@ func TestLayersTimeout(t *testing.T) { XTestLayersTimeout(t) } func TestCancelRemoves(t *testing.T) { XTestCancelRemoves(t) } func TestWithCancelCanceledParent(t *testing.T) { XTestWithCancelCanceledParent(t) } func TestWithValueChecksKey(t *testing.T) { XTestWithValueChecksKey(t) } +func TestInvalidDerivedFail(t *testing.T) { XTestInvalidDerivedFail(t) } func TestDeadlineExceededSupportsTimeout(t *testing.T) { XTestDeadlineExceededSupportsTimeout(t) } func TestCustomContextGoroutines(t *testing.T) { XTestCustomContextGoroutines(t) }