fix(jsonparser): Fix possible JSON log line corruption caused by json parser on query path (#18056)

This PR fixes two bugs in the json parser:

1. A bug that caused the original log line to be mutated by the json parser when the extracted field was a nested field and collided with an already existing label name, such as in line `{"app":{"id": 123, "name": "bar"}}` and label `app_name="foo"`.

2. A bug that caused a wrong json path for the extracted field in the same conditions as 1, such as `[]string{"app", "name_extracted"}` for `app_name`, instead of `[]string{"app", "name"}`.

Signed-off-by: Christian Haudum <[email protected]>

Author: chaudum

pkg/logql/log/parser.go

@@ -170,7 +170,9 @@ func (j *JSONParser) parseLabelValue(key, value []byte, dataType jsonparser.Valu
 	sanitized := j.buildSanitizedPrefixFromBuffer()
 	keyString, ok := j.keys.Get(sanitized, func() (string, bool) {
 		if j.lbs.BaseHas(string(sanitized)) {
-			j.prefixBuffer[prefixLen] = append(key, duplicateSuffix...)
+			j.prefixBuffer[prefixLen] = make([]byte, 0, len(key)+len(duplicateSuffix))
+			j.prefixBuffer[prefixLen] = append(j.prefixBuffer[prefixLen], key...)
+			j.prefixBuffer[prefixLen] = append(j.prefixBuffer[prefixLen], duplicateSuffix...)
 		}
 
 		keyPrefix := j.buildSanitizedPrefixFromBuffer()
@@ -182,8 +184,9 @@ func (j *JSONParser) parseLabelValue(key, value []byte, dataType jsonparser.Valu
 	})
 
 	if j.captureJSONPath {
-		jsonPath := j.buildJSONPathFromPrefixBuffer()
-		j.lbs.SetJSONPath(keyString, jsonPath)
+		if jsonPath := j.buildJSONPathFromPrefixBuffer(); len(jsonPath) > 0 {
+			j.lbs.SetJSONPath(keyString, jsonPath)
+		}
 	}
 
 	// reset the prefix position
@@ -217,10 +220,20 @@ func (j *JSONParser) buildSanitizedPrefixFromBuffer() []byte {
 	return j.sanitizedPrefixBuffer
 }
 
+// buildJSONPathFromPrefixBuffer constructs the JSON path from the accumulated prefix buffer.
+// It returns a slice of strings representing each segment of the JSON path.
+// If the prefix buffer is empty, it returns nil.
+// The function also removes any "_extracted" suffix from "duplicate" fields.
 func (j *JSONParser) buildJSONPathFromPrefixBuffer() []string {
+	if len(j.prefixBuffer) == 0 {
+		return nil
+	}
+
 	jsonPath := make([]string, 0, len(j.prefixBuffer))
 	for _, part := range j.prefixBuffer {
 		partStr := unsafe.String(unsafe.SliceData(part), len(part)) // #nosec G103 -- we know the string is not mutated
+		// Trim _extracted suffix if the extracted field was a duplicate field
+		partStr = strings.TrimSuffix(partStr, duplicateSuffix)
 		jsonPath = append(jsonPath, partStr)
 	}
 

pkg/logql/log/parser_test.go

@@ -36,6 +36,27 @@ func Test_jsonParser_Parse(t *testing.T) {
 			},
 			NoParserHints(),
 		},
+		{
+			"multi depth with duplicates",
+			[]byte(`{"app":"bar","namespace":"prod","pod":{"uuid":"bar","deployment":{"ref":"foobar"}}}`),
+			labels.FromStrings("app", "foo",
+				"pod_uuid", "foo",
+			),
+			labels.FromStrings("app", "foo",
+				"app_extracted", "bar",
+				"namespace", "prod",
+				"pod_uuid", "foo",
+				"pod_uuid_extracted", "bar",
+				"pod_deployment_ref", "foobar",
+			),
+			map[string][]string{
+				"app_extracted":      {"app"},
+				"namespace":          {"namespace"},
+				"pod_uuid_extracted": {"pod", "uuid"},
+				"pod_deployment_ref": {"pod", "deployment", "ref"},
+			},
+			NoParserHints(),
+		},
 		{"numeric",
 			[]byte(`{"counter":1, "price": {"_net_":5.56909}}`),
 			labels.EmptyLabels(),
@@ -193,10 +214,14 @@ func Test_jsonParser_Parse(t *testing.T) {
 	for _, tt := range tests {
 		j := NewJSONParser(true)
 		t.Run(tt.name, func(t *testing.T) {
+			origLine := string(tt.line)
+
 			b := NewBaseLabelsBuilderWithGrouping(nil, tt.hints, false, false).ForLabels(tt.lbs, tt.lbs.Hash())
 			b.Reset()
 			_, _ = j.Process(0, tt.line, b)
-			require.Equal(t, tt.want, b.LabelsResult().Labels())
+			labels := b.LabelsResult().Labels()
+			require.Equal(t, origLine, string(tt.line), "original log line was modified")
+			require.Equal(t, tt.want, labels)
 
 			// Check JSON paths if provided
 			if len(tt.wantJSONPath) > 0 {