From 179b2155d1a108195a1db05c84bba541ddac4a6c Mon Sep 17 00:00:00 2001
From: Julius Volz <julius.volz@gmail.com>
Date: Fri, 2 Jul 2021 10:38:14 +0200
Subject: [PATCH] Fix: Use json.Unmarshal() instead of json.Decoder (#9033)

* Fix: Use json.Unmarshal() instead of json.Decoder

See https://ahmet.im/blog/golang-json-decoder-pitfalls/

json.Decoder is for JSON streams, not single JSON objects / bodies.

Signed-off-by: Julius Volz <julius.volz@gmail.com>

* Revert modifications to targetgroup parsing

Signed-off-by: Julius Volz <julius.volz@gmail.com>
---
 discovery/hetzner/robot.go                    |  7 ++++++-
 discovery/marathon/marathon.go                |  7 ++++++-
 .../examples/custom-sd/adapter-usage/main.go  | 21 +++++++++++++++----
 notifier/notifier_test.go                     | 18 ++++++++++++++--
 4 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/discovery/hetzner/robot.go b/discovery/hetzner/robot.go
index e6f103286..f7079d909 100644
--- a/discovery/hetzner/robot.go
+++ b/discovery/hetzner/robot.go
@@ -92,8 +92,13 @@ func (d *robotDiscovery) refresh(ctx context.Context) ([]*targetgroup.Group, err
 		return nil, errors.Errorf("non 2xx status '%d' response during hetzner service discovery with role robot", resp.StatusCode)
 	}
 
+	b, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
 	var servers serversList
-	err = json.NewDecoder(resp.Body).Decode(&servers)
+	err = json.Unmarshal(b, &servers)
 	if err != nil {
 		return nil, err
 	}
diff --git a/discovery/marathon/marathon.go b/discovery/marathon/marathon.go
index ff9c76961..586245bc4 100644
--- a/discovery/marathon/marathon.go
+++ b/discovery/marathon/marathon.go
@@ -339,8 +339,13 @@ func fetchApps(ctx context.Context, client *http.Client, url string) (*appList,
 		return nil, errors.Errorf("non 2xx status '%v' response during marathon service discovery", resp.StatusCode)
 	}
 
+	b, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
 	var apps appList
-	err = json.NewDecoder(resp.Body).Decode(&apps)
+	err = json.Unmarshal(b, &apps)
 	if err != nil {
 		return nil, errors.Wrapf(err, "%q", url)
 	}
diff --git a/documentation/examples/custom-sd/adapter-usage/main.go b/documentation/examples/custom-sd/adapter-usage/main.go
index 6a50c38ae..2214a823d 100644
--- a/documentation/examples/custom-sd/adapter-usage/main.go
+++ b/documentation/examples/custom-sd/adapter-usage/main.go
@@ -100,13 +100,17 @@ func (d *discovery) parseServiceNodes(resp *http.Response, name string) (*target
 		Labels: make(model.LabelSet),
 	}
 
-	dec := json.NewDecoder(resp.Body)
 	defer func() {
 		io.Copy(ioutil.Discard, resp.Body)
 		resp.Body.Close()
 	}()
-	err := dec.Decode(&nodes)
 
+	b, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	err = json.Unmarshal(b, &nodes)
 	if err != nil {
 		return &tgroup, err
 	}
@@ -165,8 +169,8 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
 			continue
 		}
 
-		dec := json.NewDecoder(resp.Body)
-		err = dec.Decode(&srvs)
+		b, err := ioutil.ReadAll(resp.Body)
+		io.Copy(ioutil.Discard, resp.Body)
 		resp.Body.Close()
 		if err != nil {
 			level.Error(d.logger).Log("msg", "Error reading services list", "err", err)
@@ -174,6 +178,14 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
 			continue
 		}
 
+		err = json.Unmarshal(b, &srvs)
+		resp.Body.Close()
+		if err != nil {
+			level.Error(d.logger).Log("msg", "Error parsing services list", "err", err)
+			time.Sleep(time.Duration(d.refreshInterval) * time.Second)
+			continue
+		}
+
 		var tgs []*targetgroup.Group
 		// Note that we treat errors when querying specific consul services as fatal for this
 		// iteration of the time.Tick loop. It's better to have some stale targets than an incomplete
@@ -191,6 +203,7 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
 				level.Error(d.logger).Log("msg", "Error getting services nodes", "service", name, "err", err)
 				break
 			}
+
 			tg, err := d.parseServiceNodes(resp, name)
 			if err != nil {
 				level.Error(d.logger).Log("msg", "Error parsing services nodes", "service", name, "err", err)
diff --git a/notifier/notifier_test.go b/notifier/notifier_test.go
index 0e66f826c..1e7854ba1 100644
--- a/notifier/notifier_test.go
+++ b/notifier/notifier_test.go
@@ -125,8 +125,16 @@ func TestHandlerSendAll(t *testing.T) {
 				w.WriteHeader(http.StatusInternalServerError)
 				return
 			}
+
+			b, err := ioutil.ReadAll(r.Body)
+			if err != nil {
+				err = errors.Errorf("error reading body: %v", err)
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+
 			var alerts []*Alert
-			err = json.NewDecoder(r.Body).Decode(&alerts)
+			err = json.Unmarshal(b, &alerts)
 			if err == nil {
 				err = alertsEqual(expected, alerts)
 			}
@@ -322,7 +330,13 @@ func TestHandlerQueuing(t *testing.T) {
 		select {
 		case expected := <-expectedc:
 			var alerts []*Alert
-			err := json.NewDecoder(r.Body).Decode(&alerts)
+
+			b, err := ioutil.ReadAll(r.Body)
+			if err != nil {
+				panic(err)
+			}
+
+			err = json.Unmarshal(b, &alerts)
 			if err == nil {
 				err = alertsEqual(expected, alerts)
 			}