From b96718f8f0fc4ba31e7662e3308f808685f4cf36 Mon Sep 17 00:00:00 2001
From: dfawley <dfawley@google.com>
Date: Mon, 19 Mar 2018 16:23:41 -0700
Subject: [PATCH] metadata: Fix bug where AppendToOutgoingContext could modify
 another context's metadata (#1930)

---
 metadata/metadata.go      |  8 +++++--
 metadata/metadata_test.go | 48 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/metadata/metadata.go b/metadata/metadata.go
index 15662b5d..e7c99467 100644
--- a/metadata/metadata.go
+++ b/metadata/metadata.go
@@ -131,7 +131,11 @@ func AppendToOutgoingContext(ctx context.Context, kv ...string) context.Context
 		panic(fmt.Sprintf("metadata: AppendToOutgoingContext got an odd number of input pairs for metadata: %d", len(kv)))
 	}
 	md, _ := ctx.Value(mdOutgoingKey{}).(rawMD)
-	return context.WithValue(ctx, mdOutgoingKey{}, rawMD{md: md.md, added: append(md.added, kv)})
+	added := make([][]string, len(md.added)+1)
+	copy(added, md.added)
+	added[len(added)-1] = make([]string, len(kv))
+	copy(added[len(added)-1], kv)
+	return context.WithValue(ctx, mdOutgoingKey{}, rawMD{md: md.md, added: added})
 }
 
 // FromIncomingContext returns the incoming metadata in ctx if it exists.  The
@@ -159,7 +163,7 @@ func FromOutgoingContextRaw(ctx context.Context) (MD, [][]string, bool) {
 
 // FromOutgoingContext returns the outgoing metadata in ctx if it exists.  The
 // returned MD should not be modified. Writing to it may cause races.
-// Modification should be made to the copies of the returned MD.
+// Modification should be made to copies of the returned MD.
 func FromOutgoingContext(ctx context.Context) (MD, bool) {
 	raw, ok := ctx.Value(mdOutgoingKey{}).(rawMD)
 	if !ok {
diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go
index 6edeeda6..526e0b6c 100644
--- a/metadata/metadata_test.go
+++ b/metadata/metadata_test.go
@@ -20,6 +20,7 @@ package metadata
 
 import (
 	"reflect"
+	"strconv"
 	"testing"
 
 	"golang.org/x/net/context"
@@ -98,19 +99,60 @@ func TestAppendToOutgoingContext(t *testing.T) {
 	}
 }
 
+func TestAppendToOutgoingContext_Repeated(t *testing.T) {
+	ctx := context.Background()
+
+	for i := 0; i < 100; i = i + 2 {
+		ctx1 := AppendToOutgoingContext(ctx, "k", strconv.Itoa(i))
+		ctx2 := AppendToOutgoingContext(ctx, "k", strconv.Itoa(i+1))
+
+		md1, _ := FromOutgoingContext(ctx1)
+		md2, _ := FromOutgoingContext(ctx2)
+
+		if reflect.DeepEqual(md1, md2) {
+			t.Fatalf("md1, md2 = %v, %v; should not be equal", md1, md2)
+		}
+
+		ctx = ctx1
+	}
+}
+
+func TestAppendToOutgoingContext_FromKVSlice(t *testing.T) {
+	const k, v = "a", "b"
+	kv := []string{k, v}
+	ctx := AppendToOutgoingContext(context.Background(), kv...)
+	md, _ := FromOutgoingContext(ctx)
+	if md[k][0] != v {
+		t.Fatalf("md[%q] = %q; want %q", k, md[k], v)
+	}
+	kv[1] = "xxx"
+	md, _ = FromOutgoingContext(ctx)
+	if md[k][0] != v {
+		t.Fatalf("md[%q] = %q; want %q", k, md[k], v)
+	}
+}
+
 // Old/slow approach to adding metadata to context
 func Benchmark_AddingMetadata_ContextManipulationApproach(b *testing.B) {
+	// TODO: Add in N=1-100 tests once Go1.6 support is removed.
+	const num = 10
 	for n := 0; n < b.N; n++ {
 		ctx := context.Background()
-		md, _ := FromOutgoingContext(ctx)
-		NewOutgoingContext(ctx, Join(Pairs("k1", "v1", "k2", "v2"), md))
+		for i := 0; i < num; i++ {
+			md, _ := FromOutgoingContext(ctx)
+			NewOutgoingContext(ctx, Join(Pairs("k1", "v1", "k2", "v2"), md))
+		}
 	}
 }
 
 // Newer/faster approach to adding metadata to context
 func BenchmarkAppendToOutgoingContext(b *testing.B) {
+	const num = 10
 	for n := 0; n < b.N; n++ {
-		AppendToOutgoingContext(context.Background(), "k1", "v1", "k2", "v2")
+		ctx := context.Background()
+		for i := 0; i < num; i++ {
+			ctx = AppendToOutgoingContext(ctx, "k1", "v1", "k2", "v2")
+		}
 	}
 }