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>
This commit is contained in:
Julius Volz 2021-07-02 10:38:14 +02:00 committed by GitHub
parent f72cabb437
commit 179b2155d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 45 additions and 8 deletions

View File

@ -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) 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 var servers serversList
err = json.NewDecoder(resp.Body).Decode(&servers) err = json.Unmarshal(b, &servers)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -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) 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 var apps appList
err = json.NewDecoder(resp.Body).Decode(&apps) err = json.Unmarshal(b, &apps)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "%q", url) return nil, errors.Wrapf(err, "%q", url)
} }

View File

@ -100,13 +100,17 @@ func (d *discovery) parseServiceNodes(resp *http.Response, name string) (*target
Labels: make(model.LabelSet), Labels: make(model.LabelSet),
} }
dec := json.NewDecoder(resp.Body)
defer func() { defer func() {
io.Copy(ioutil.Discard, resp.Body) io.Copy(ioutil.Discard, resp.Body)
resp.Body.Close() 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 { if err != nil {
return &tgroup, err return &tgroup, err
} }
@ -165,8 +169,8 @@ func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
continue continue
} }
dec := json.NewDecoder(resp.Body) b, err := ioutil.ReadAll(resp.Body)
err = dec.Decode(&srvs) io.Copy(ioutil.Discard, resp.Body)
resp.Body.Close() resp.Body.Close()
if err != nil { if err != nil {
level.Error(d.logger).Log("msg", "Error reading services list", "err", err) 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 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 var tgs []*targetgroup.Group
// Note that we treat errors when querying specific consul services as fatal for this // 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 // 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) level.Error(d.logger).Log("msg", "Error getting services nodes", "service", name, "err", err)
break break
} }
tg, err := d.parseServiceNodes(resp, name) tg, err := d.parseServiceNodes(resp, name)
if err != nil { if err != nil {
level.Error(d.logger).Log("msg", "Error parsing services nodes", "service", name, "err", err) level.Error(d.logger).Log("msg", "Error parsing services nodes", "service", name, "err", err)

View File

@ -125,8 +125,16 @@ func TestHandlerSendAll(t *testing.T) {
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)
return 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 var alerts []*Alert
err = json.NewDecoder(r.Body).Decode(&alerts) err = json.Unmarshal(b, &alerts)
if err == nil { if err == nil {
err = alertsEqual(expected, alerts) err = alertsEqual(expected, alerts)
} }
@ -322,7 +330,13 @@ func TestHandlerQueuing(t *testing.T) {
select { select {
case expected := <-expectedc: case expected := <-expectedc:
var alerts []*Alert 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 { if err == nil {
err = alertsEqual(expected, alerts) err = alertsEqual(expected, alerts)
} }