Merge pull request #610 from iamemilio/encode_bug

Defend against encoding bug until we solve it

Author: iamemilio

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{}
 

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
 				}

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(),