From d0ad532fa17706c86ab1eaa4b0f03dc8ac63f6a6 Mon Sep 17 00:00:00 2001
From: Wangchong Zhou <fffonion@gmail.com>
Date: Wed, 5 Dec 2018 11:25:43 -0800
Subject: [PATCH] fix: don't let captured fields being overwritten by
 backtracking

Signed-off-by: Wangchong Zhou <fffonion@gmail.com>
---
 pkg/mapper/fsm/fsm.go     |  6 ++++--
 pkg/mapper/mapper_test.go | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/pkg/mapper/fsm/fsm.go b/pkg/mapper/fsm/fsm.go
index 263dba8..efa22c5 100644
--- a/pkg/mapper/fsm/fsm.go
+++ b/pkg/mapper/fsm/fsm.go
@@ -138,6 +138,7 @@ func (f *FSM) GetMapping(statsdMetric string, statsdMetricType string) (*mapping
 	var finalState *mappingState
 
 	captures := make([]string, len(matchFields))
+	finalCaptures := make([]string, len(matchFields))
 	// keep track of captured group so we don't need to do append() on captures
 	captureIdx := 0
 	filedsCount := len(matchFields)
@@ -193,6 +194,8 @@ func (f *FSM) GetMapping(statsdMetric string, statsdMetricType string) (*mapping
 				} else if finalState == nil || finalState.ResultPriority > state.ResultPriority {
 					// if we care about ordering, try to find a result with highest prioity
 					finalState = state
+					// do a deep copy to preserve current captures
+					copy(finalCaptures, captures)
 				}
 				break
 			}
@@ -224,8 +227,7 @@ func (f *FSM) GetMapping(statsdMetric string, statsdMetricType string) (*mapping
 			resumeFromBacktrack = true
 		}
 	}
-
-	return finalState, captures
+	return finalState, finalCaptures
 }
 
 // TestIfNeedBacktracking tests if backtrack is needed for given list of mappings
diff --git a/pkg/mapper/mapper_test.go b/pkg/mapper/mapper_test.go
index 64fb73d..08e4306 100644
--- a/pkg/mapper/mapper_test.go
+++ b/pkg/mapper/mapper_test.go
@@ -167,6 +167,50 @@ mappings:
 						"label": "justatest_foo",
 					},
 				},
+				"backtrack.justatest.aaa": {
+					name: "testa",
+					labels: map[string]string{
+						"label": "_foo",
+					},
+				},
+			},
+		},
+		//Config with backtracking, the non-matched rule has star(s)
+		// A metric like full.name.anothertest will first match full.name.* and then tries
+		// to match *.dummy.* and then failed.
+		// This test case makes sure the captures in the non-matched later rule
+		// doesn't affect the captures in the first matched rule.
+		{
+			config: `
+defaults:
+  glob_disable_ordering: false
+mappings:
+- match: '*.dummy.*'
+  name: metric_one
+  labels:
+    system: $1
+    attribute: $2
+- match: 'full.name.*'
+  name: metric_two
+  labels:
+    system: static
+    attribute: $1
+`,
+			mappings: mappings{
+				"whatever.dummy.test": {
+					name: "metric_one",
+					labels: map[string]string{
+						"system":    "whatever",
+						"attribute": "test",
+					},
+				},
+				"full.name.anothertest": {
+					name: "metric_two",
+					labels: map[string]string{
+						"system":    "static",
+						"attribute": "anothertest",
+					},
+				},
 			},
 		},
 		//Config with super sets, disables ordering