From 0d3bd975a6cae3a93eb28848e459c987b0643b0e Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Fri, 2 Dec 2022 16:20:37 -0500 Subject: [PATCH 1/2] 99% test coverage for encode.go --- v3/internal/jsonx/encode_test.go | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/v3/internal/jsonx/encode_test.go b/v3/internal/jsonx/encode_test.go index cc0f5934c..ef5ef38b0 100644 --- a/v3/internal/jsonx/encode_test.go +++ b/v3/internal/jsonx/encode_test.go @@ -5,6 +5,7 @@ package jsonx import ( "bytes" + "fmt" "math" "testing" ) @@ -28,6 +29,30 @@ func TestAppendFloat(t *testing.T) { } } +func TestAppendFloat32(t *testing.T) { + buf := &bytes.Buffer{} + + err := AppendFloat32(buf, float32(math.NaN())) + if err == nil { + t.Error("AppendFloat(NaN) should return an error") + } + + err = AppendFloat32(buf, float32(math.Inf(1))) + if err == nil { + t.Error("AppendFloat(+Inf) should return an error") + } + + err = AppendFloat32(buf, float32(math.Inf(-1))) + if err == nil { + t.Error("AppendFloat(-Inf) should return an error") + } + + err = AppendFloat32(buf, float32(12.5)) + if err != nil { + t.Error("AppendFloat(12.5) should not return an error") + } +} + func TestAppendFloats(t *testing.T) { buf := &bytes.Buffer{} @@ -166,6 +191,15 @@ var encodeStringTests = []struct { {"\\", `"\\"`}, {`"`, `"\""`}, {"the\u2028quick\t\nbrown\u2029fox", `"the\u2028quick\t\nbrown\u2029fox"`}, + + //extra edge cases + {string([]byte{237, 159, 193}), `"\ufffd\ufffd\ufffd"`}, // invalid utf8 + {string([]byte{55, 237, 159, 193, 55}), `"7\ufffd\ufffd\ufffd7"`}, // invalid utf8 surrounded by valid utf8 + {`abcdefghijklmnopqrstuvwxyz1234567890`, `"abcdefghijklmnopqrstuvwxyz1234567890"`}, // alphanumeric + {"'", `"'"`}, + {``, `""`}, + {`\`, `"\\"`}, + {fmt.Sprintf("%c", rune(65533)), fmt.Sprintf("\"%c\"", rune(65533))}, // invalid rune utf8 symbol (valid utf8) } func TestAppendString(t *testing.T) { @@ -181,6 +215,41 @@ func TestAppendString(t *testing.T) { } } +func TestAppendStringArray(t *testing.T) { + buf := &bytes.Buffer{} + + var encodeStringArrayTests = []struct { + in []string + out string + }{ + { + in: []string{ + "hi", + "foo", + }, + out: `["hi","foo"]`, + }, + { + in: []string{ + "foo", + }, + out: `["foo"]`, + }, + { + in: []string{}, + out: `[]`, + }, + } + + for _, tt := range encodeStringArrayTests { + buf.Reset() + + AppendStringArray(buf, tt.in...) + if got := buf.String(); got != tt.out { + t.Errorf("AppendString(%q) = %#q, want %#q", tt.in, got, tt.out) + } + } +} func BenchmarkAppendString(b *testing.B) { buf := &bytes.Buffer{} From 09541bf52f2730eec0b0c1e298720aa9a639a6aa Mon Sep 17 00:00:00 2001 From: Emilio Garcia Date: Mon, 5 Dec 2022 11:00:49 -0500 Subject: [PATCH 2/2] guard payload harvest from panic --- v3/newrelic/attributes_from_internal.go | 10 ++++------ v3/newrelic/internal_app.go | 20 +++++++++++++++++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/v3/newrelic/attributes_from_internal.go b/v3/newrelic/attributes_from_internal.go index 96d1e829c..28e8b0c37 100644 --- a/v3/newrelic/attributes_from_internal.go +++ b/v3/newrelic/attributes_from_internal.go @@ -275,10 +275,8 @@ func (attr agentAttributes) Add(id string, stringVal string, otherVal interface{ } } -// // Remove is used to remove agent attributes. // It is not an error if the attribute wasn't present to begin with. -// func (attr agentAttributes) Remove(id string) { if _, ok := attr[id]; ok { delete(attr, id) @@ -453,14 +451,14 @@ func writeAttributeValueJSON(w *jsonFieldsWriter, key string, val interface{}) { } func agentAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet) { - if nil == a { + if a == nil { buf.WriteString("{}") return } w := jsonFieldsWriter{buf: buf} buf.WriteByte('{') for id, val := range a.Agent { - if 0 != a.config.agentDests[id]&d { + if a.config.agentDests[id]&d != 0 { if val.stringVal != "" { w.stringField(id, val.stringVal) } else { @@ -478,12 +476,12 @@ func userAttributesJSON(a *attributes, buf *bytes.Buffer, d destinationSet, extr w := jsonFieldsWriter{buf: buf} for key, val := range extraAttributes { outputDest := applyAttributeConfig(a.config, key, d) - if 0 != outputDest&d { + if outputDest&d != 0 { writeAttributeValueJSON(&w, key, val) } } for name, atr := range a.user { - if 0 != atr.dests&d { + if atr.dests&d != 0 { if _, found := extraAttributes[name]; found { continue } diff --git a/v3/newrelic/internal_app.go b/v3/newrelic/internal_app.go index 71042f2c5..1d8b9d474 100644 --- a/v3/newrelic/internal_app.go +++ b/v3/newrelic/internal_app.go @@ -71,16 +71,30 @@ func (app *app) doHarvest(h *harvest, harvestStart time.Time, run *appRun) { payloads := h.Payloads(app.config.DistributedTracer.Enabled) for _, p := range payloads { cmd := p.EndpointMethod() + var data []byte + + defer func() { + if r := recover(); r != nil { + app.Warn("panic occured when creating harvest data", map[string]interface{}{ + "cmd": cmd, + "panic": r, + }) + + // make sure the loop continues + data = nil + } + }() + data, err := p.Data(run.Reply.RunID.String(), harvestStart) - if nil != err { + if err != nil { app.Warn("unable to create harvest data", map[string]interface{}{ "cmd": cmd, "error": err.Error(), }) continue } - if nil == data { + if data == nil { continue } @@ -103,7 +117,7 @@ func (app *app) doHarvest(h *harvest, harvestStart time.Time, run *appRun) { return } - if nil != resp.Err { + if resp.Err != nil { app.Warn("harvest failure", map[string]interface{}{ "cmd": cmd, "error": resp.Err.Error(),