ddtracer/tracer: marshalled span values in encode span (#832)

Illegal characters, such as newlines, were not escaped in encodeSpan function, which ruined JSON structure

Closes #791

ddtrace/tracer/writer.go

@@ -7,6 +7,7 @@ package tracer
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"io"
 	"math"
@@ -173,25 +174,23 @@ func (h *logTraceWriter) encodeSpan(s *span) {
 	h.buf.Write(strconv.AppendUint(scratch[:0], uint64(s.SpanID), 16))
 	h.buf.WriteString(`","parent_id":"`)
 	h.buf.Write(strconv.AppendUint(scratch[:0], uint64(s.ParentID), 16))
-	h.buf.WriteString(`","name":"`)
-	h.buf.WriteString(s.Name)
-	h.buf.WriteString(`","resource":"`)
-	h.buf.WriteString(s.Resource)
-	h.buf.WriteString(`","error":`)
+	h.buf.WriteString(`","name":`)
+	h.marshalString(s.Name)
+	h.buf.WriteString(`,"resource":`)
+	h.marshalString(s.Resource)
+	h.buf.WriteString(`,"error":`)
 	h.buf.Write(strconv.AppendInt(scratch[:0], int64(s.Error), 10))
 	h.buf.WriteString(`,"meta":{`)
 	first := true
 	for k, v := range s.Meta {
 		if first {
-			h.buf.WriteByte('"')
 			first = false
 		} else {
-			h.buf.WriteString(`,"`)
+			h.buf.WriteString(`,`)
 		}
-		h.buf.WriteString(k)
-		h.buf.WriteString(`":"`)
-		h.buf.WriteString(v)
-		h.buf.WriteString(`"`)
+		h.marshalString(k)
+		h.buf.WriteString(":")
+		h.marshalString(v)
 	}
 	h.buf.WriteString(`},"metrics":{`)
 	first = true
@@ -201,22 +200,32 @@ func (h *logTraceWriter) encodeSpan(s *span) {
 			continue
 		}
 		if first {
-			h.buf.WriteByte('"')
 			first = false
 		} else {
-			h.buf.WriteString(`,"`)
+			h.buf.WriteString(`,`)
 		}
-		h.buf.WriteString(k)
-		h.buf.WriteString(`":`)
+		h.marshalString(k)
+		h.buf.WriteString(`:`)
 		h.buf.Write(encodeFloat(scratch[:0], v))
 	}
 	h.buf.WriteString(`},"start":`)
 	h.buf.Write(strconv.AppendInt(scratch[:0], s.Start, 10))
 	h.buf.WriteString(`,"duration":`)
 	h.buf.Write(strconv.AppendInt(scratch[:0], s.Duration, 10))
-	h.buf.WriteString(`,"service":"`)
-	h.buf.WriteString(s.Service)
-	h.buf.WriteString(`"}`)
+	h.buf.WriteString(`,"service":`)
+	h.marshalString(s.Service)
+	h.buf.WriteString(`}`)
+}
+
+// marshalString marshals the string str as JSON into the writer's buffer.
+// Should be used whenever writing non-constant string data to ensure correct sanitization.
+func (h *logTraceWriter) marshalString(str string) {
+	m, err := json.Marshal(str)
+	if err != nil {
+		log.Error("Error marshaling value %q: %v", str, err)
+	} else {
+		h.buf.Write(m)
+	}
 }
 
 type encodingError struct {

ddtrace/tracer/writer_test.go

@@ -204,6 +204,22 @@ func TestLogWriter(t *testing.T) {
 		assert.NoError(err)
 		assert.Equal(jsonPayload{[][]jsonSpan{{expected}}}, payload)
 	})
+
+	t.Run("invalid-characters", func(t *testing.T) {
+		assert := assert.New(t)
+		s := newSpan("name\n", "srv\t", `"res"`, 2, 1, 3)
+		s.Start = 12
+		s.Meta["query\n"] = "Select * from \n Where\nvalue"
+		s.Metrics["version\n"] = 3
+
+		var w logTraceWriter
+		w.encodeSpan(s)
+
+		str := w.buf.String()
+		assert.Equal(`{"trace_id":"1","span_id":"2","parent_id":"3","name":"name\n","resource":"\"res\"","error":0,"meta":{"query\n":"Select * from \n Where\nvalue"},"metrics":{"version\n":3},"start":12,"duration":0,"service":"srv\t"}`, str)
+		assert.NotContains(str, "\n")
+		assert.Contains(str, "\\n")
+	})
 }
 
 func TestLogWriterOverflow(t *testing.T) {