Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defend against encoding bug until we solve it #610

Merged
merged 2 commits into from Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 69 additions & 0 deletions v3/internal/jsonx/encode_test.go
Expand Up @@ -5,6 +5,7 @@ package jsonx

import (
"bytes"
"fmt"
"math"
"testing"
)
Expand All @@ -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{}

Expand Down Expand Up @@ -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) {
Expand All @@ -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{}

Expand Down
10 changes: 4 additions & 6 deletions v3/newrelic/attributes_from_internal.go
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down
20 changes: 17 additions & 3 deletions v3/newrelic/internal_app.go
Expand Up @@ -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
}

Expand All @@ -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(),
Expand Down