Skip to content

Commit

Permalink
Merge pull request #169 from Kong/fix-backtracking-captures-being-ove…
Browse files Browse the repository at this point in the history
…rwritten

fix: don't let captured fields being overwritten by backtracking
  • Loading branch information
Matthias Rampke authored Dec 5, 2018
2 parents 2755595 + d0ad532 commit e019c01
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
6 changes: 4 additions & 2 deletions pkg/mapper/fsm/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
44 changes: 44 additions & 0 deletions pkg/mapper/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e019c01

Please sign in to comment.