From 261586b86217660d4e0d4ee7a0b048036d9e57cf Mon Sep 17 00:00:00 2001
From: Andrew Seigner <andrew@sig.gy>
Date: Sat, 10 Feb 2018 11:04:28 -0800
Subject: [PATCH] Fix pointer copying (#330)

The Public APIs stat endpoint copies a slice of values to a slice of
pointers prior to gRPC response. Go's range clause re-uses the same
pointer for each iteration of the loop, causing a slice of {1,2,3}
becoming {3,3,3}.

Fix the range loop to directly reference pointers in the slice of
values, ignoring the range variable. Also add tests to catch this case.

Signed-off-by: Andrew Seigner <siggy@buoyant.io>
---
 controller/api/public/grpc_server.go      |  4 +--
 controller/api/public/grpc_server_test.go | 32 +++++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/controller/api/public/grpc_server.go b/controller/api/public/grpc_server.go
index 14507d991..6eae72818 100644
--- a/controller/api/public/grpc_server.go
+++ b/controller/api/public/grpc_server.go
@@ -107,8 +107,8 @@ func (s *grpcServer) Stat(ctx context.Context, req *pb.MetricRequest) (*pb.Metri
 			log.Errorf("Stat -> queryMetric failed with: %s", err)
 			err = result.err
 		} else {
-			for _, ser := range result.series {
-				metrics = append(metrics, &ser)
+			for i := range result.series {
+				metrics = append(metrics, &result.series[i])
 			}
 		}
 	}
diff --git a/controller/api/public/grpc_server_test.go b/controller/api/public/grpc_server_test.go
index cd5a08f84..86c287002 100644
--- a/controller/api/public/grpc_server_test.go
+++ b/controller/api/public/grpc_server_test.go
@@ -41,12 +41,23 @@ func TestStat(t *testing.T) {
 						&telemetry.Sample{
 							Values: []*telemetry.SampleValue{
 								&telemetry.SampleValue{Value: 1, TimestampMs: 2},
+								&telemetry.SampleValue{Value: 3, TimestampMs: 4},
 							},
 							Labels: map[string]string{
 								sourceDeployLabel: "sourceDeployLabel",
 								targetDeployLabel: "targetDeployLabel",
 							},
 						},
+						&telemetry.Sample{
+							Values: []*telemetry.SampleValue{
+								&telemetry.SampleValue{Value: 5, TimestampMs: 6},
+								&telemetry.SampleValue{Value: 7, TimestampMs: 8},
+							},
+							Labels: map[string]string{
+								sourceDeployLabel: "sourceDeployLabel2",
+								targetDeployLabel: "targetDeployLabel2",
+							},
+						},
 					},
 				},
 				mReq: &pb.MetricRequest{
@@ -67,6 +78,27 @@ func TestStat(t *testing.T) {
 									Value:       &pb.MetricValue{Value: &pb.MetricValue_Gauge{Gauge: 1}},
 									TimestampMs: 2,
 								},
+								&pb.MetricDatapoint{
+									Value:       &pb.MetricValue{Value: &pb.MetricValue_Gauge{Gauge: 3}},
+									TimestampMs: 4,
+								},
+							},
+						},
+						&pb.MetricSeries{
+							Name: pb.MetricName_REQUEST_RATE,
+							Metadata: &pb.MetricMetadata{
+								SourceDeploy: "sourceDeployLabel2",
+								TargetDeploy: "targetDeployLabel2",
+							},
+							Datapoints: []*pb.MetricDatapoint{
+								&pb.MetricDatapoint{
+									Value:       &pb.MetricValue{Value: &pb.MetricValue_Gauge{Gauge: 5}},
+									TimestampMs: 6,
+								},
+								&pb.MetricDatapoint{
+									Value:       &pb.MetricValue{Value: &pb.MetricValue_Gauge{Gauge: 7}},
+									TimestampMs: 8,
+								},
 							},
 						},
 					},