From 69686af32b00723abf71f2d27300130c3011cc95 Mon Sep 17 00:00:00 2001 From: dskoval <44838975+dskoval@users.noreply.github.com> Date: Thu, 8 Nov 2018 17:02:00 +0200 Subject: [PATCH 1/2] Add access to regexp capturing group Inside URL rewriter - add additional level for $tyc_context trigger keys - $tyk_context.trigger-{n}-{name}-{i}-{groupNum} holds group value Fixes https://github.com/TykTechnologies/tyk/issues/1833 --- apidef/api_definitions.go | 8 +++ mw_url_rewrite.go | 85 +++++++++++++++------- mw_url_rewrite_test.go | 144 +++++++++++++++++++++++++++++++++++++- 3 files changed, 212 insertions(+), 25 deletions(-) diff --git a/apidef/api_definitions.go b/apidef/api_definitions.go index 4a54845ee613..04cf66e72028 100644 --- a/apidef/api_definitions.go +++ b/apidef/api_definitions.go @@ -540,6 +540,14 @@ func (s *StringRegexMap) Check(value string) string { return s.matchRegex.FindString(value) } +func (s *StringRegexMap) FindStringSubmatch(value string) []string { + return s.matchRegex.FindStringSubmatch(value) +} + +func (s *StringRegexMap) FindAllStringSubmatch(value string, n int) [][]string { + return s.matchRegex.FindAllStringSubmatch(value, n) +} + func (s *StringRegexMap) Init() error { var err error if s.matchRegex, err = regexp.Compile(s.MatchPattern); err != nil { diff --git a/mw_url_rewrite.go b/mw_url_rewrite.go index 5424eaf94248..2a8913c27f6a 100644 --- a/mw_url_rewrite.go +++ b/mw_url_rewrite.go @@ -16,8 +16,10 @@ import ( ) const ( - metaLabel = "$tyk_meta." - contextLabel = "$tyk_context." + metaLabel = "$tyk_meta." + contextLabel = "$tyk_context." + triggerKeyPrefix = "trigger" + triggerKeySep = "-" ) var dollarMatch = regexp.MustCompile(`\$\d+`) @@ -178,7 +180,7 @@ func replaceTykVariables(r *http.Request, in string, escape bool) string { replaceGroups := contextMatch.FindAllStringSubmatch(in, -1) for _, v := range replaceGroups { - contextKey := strings.Replace(v[0], "$tyk_context.", "", 1) + contextKey := strings.Replace(v[0], contextLabel, "", 1) if val, ok := contextData[contextKey]; ok { valStr := valToStr(val) @@ -202,7 +204,7 @@ func replaceTykVariables(r *http.Request, in string, escape bool) string { replaceGroups := metaMatch.FindAllStringSubmatch(in, -1) for _, v := range replaceGroups { - contextKey := strings.Replace(v[0], "$tyk_meta.", "", 1) + contextKey := strings.Replace(v[0], metaLabel, "", 1) val, ok := session.MetaData[contextKey] if ok { @@ -362,10 +364,13 @@ func checkHeaderTrigger(r *http.Request, options map[string]apidef.StringRegexMa vals, ok := r.Header[mhCN] if ok { for i, v := range vals { - b := mr.Check(v) - if len(b) > 0 { - kn := "trigger-" + strconv.Itoa(triggernum) + "-" + mhCN + "-" + strconv.Itoa(i) - contextData[kn] = b + match := mr.FindStringSubmatch(v) + if len(match) > 0 { + kn := buildTriggerKey(triggernum, mhCN, i) + contextData[kn] = match[0] + + addGroupsToContextData(&contextData, kn, match[1:]) + fCount++ } } @@ -392,11 +397,13 @@ func checkQueryString(r *http.Request, options map[string]apidef.StringRegexMap, vals, ok := qvals[mv] if ok { for i, v := range vals { - b := mr.Check(v) - if len(b) > 0 { - kn := "trigger-" + strconv.Itoa(triggernum) + "-" + mv + "-" + strconv.Itoa(i) + match := mr.FindStringSubmatch(v) + if len(match) > 0 { + kn := buildTriggerKey(triggernum, mv, i) + contextData[kn] = match[0] + + addGroupsToContextData(&contextData, kn, match[1:]) - contextData[kn] = b fCount++ } } @@ -422,11 +429,13 @@ func checkPathParts(r *http.Request, options map[string]apidef.StringRegexMap, a pathParts := strings.Split(r.URL.Path, "/") for _, part := range pathParts { - b := mr.Check(part) - if len(b) > 0 { - kn := "trigger-" + strconv.Itoa(triggernum) + "-" + mv + "-" + strconv.Itoa(fCount) + match := mr.FindStringSubmatch(part) + if len(match) > 0 { + kn := buildTriggerKey(triggernum, mv, fCount) + contextData[kn] = match[0] + + addGroupsToContextData(&contextData, kn, match[1:]) - contextData[kn] = b fCount++ } } @@ -452,11 +461,13 @@ func checkSessionTrigger(r *http.Request, sess *user.SessionState, options map[s if ok { val, valOk := rawVal.(string) if valOk { - b := mr.Check(val) - if len(b) > 0 { - kn := "trigger-" + strconv.Itoa(triggernum) + "-" + mh + matches := mr.FindStringSubmatch(val) + if len(matches) > 0 { + kn := buildTriggerKey(triggernum, mh) + contextData[kn] = matches[0] + + addGroupsToContextData(&contextData, kn, matches[1:]) - contextData[kn] = b fCount++ } } @@ -479,13 +490,39 @@ func checkPayload(r *http.Request, options apidef.StringRegexMap, triggernum int contextData := ctxGetData(r) bodyBytes, _ := ioutil.ReadAll(r.Body) - b := options.Check(string(bodyBytes)) - if len(b) > 0 { - kn := "trigger-" + strconv.Itoa(triggernum) + "-payload" + matches := options.FindAllStringSubmatch(string(bodyBytes), -1) - contextData[kn] = string(b) + if len(matches) > 0 { + kn := buildTriggerKey(triggernum, "payload") + contextData[kn] = matches[0][0] + + for i, match := range matches { + kn = buildTriggerKey(triggernum, "payload", i) + contextData[kn] = match[0] + + addGroupsToContextData(&contextData, kn, match[1:]) + } return true } return false } + +func buildTriggerKey(num int, name string, indices ...int) string { + parts := []string{triggerKeyPrefix, strconv.Itoa(num), name} + + if len(indices) > 0 { + for _, index := range indices { + parts = append(parts, strconv.Itoa(index)) + } + } + + return strings.Join(parts, triggerKeySep) +} + +func addGroupsToContextData(cd *map[string]interface{}, keyPrefix string, groups []string) { + for i, g := range groups { + k := strings.Join([]string{keyPrefix, strconv.Itoa(i)}, triggerKeySep) + (*cd)[k] = g + } +} diff --git a/mw_url_rewrite_test.go b/mw_url_rewrite_test.go index 5c6a6166bfe3..79ed83ba4c55 100644 --- a/mw_url_rewrite_test.go +++ b/mw_url_rewrite_test.go @@ -129,6 +129,32 @@ func TestRewriterTriggers(t *testing.T) { func() TestDef { r, _ := http.NewRequest("GET", "/test/straight/rewrite", nil) + r.Header.Set("x-test", "hello-world") + + hOpt := apidef.StringRegexMap{MatchPattern: "hello-(\\w+)"} + hOpt.Init() + + return TestDef{ + "Header Single Group", + "/test/straight/rewrite", "/change/to/me/ignore", + "/test/straight/rewrite", "/change/to/me/world", + []apidef.RoutingTrigger{ + { + On: apidef.Any, + Options: apidef.RoutingTriggerOptions{ + HeaderMatches: map[string]apidef.StringRegexMap{ + "x-test": hOpt, + }, + }, + RewriteTo: "/change/to/me/$tyk_context.trigger-0-X-Test-0-0", + }, + }, + r, + } + }, + func() TestDef { + r, _ := http.NewRequest("GET", "/test/straight/rewrite", nil) + patt := "bar" r.Header.Set("x-test-Two", patt) @@ -285,6 +311,30 @@ func TestRewriterTriggers(t *testing.T) { r, } }, + func() TestDef { + r, _ := http.NewRequest("GET", "/test/query/rewrite?x_test=foo-bar", nil) + + hOpt := apidef.StringRegexMap{MatchPattern: "foo-(\\w+)"} + hOpt.Init() + + return TestDef{ + "Query Single Group", + "/test/query/rewrite", "/change/to/me/ignore", + "/test/query/rewrite", "/change/to/me/bar", + []apidef.RoutingTrigger{ + { + On: apidef.Any, + Options: apidef.RoutingTriggerOptions{ + QueryValMatches: map[string]apidef.StringRegexMap{ + "x_test": hOpt, + }, + }, + RewriteTo: "/change/to/me/$tyk_context.trigger-0-x_test-0-0", + }, + }, + r, + } + }, func() TestDef { r, _ := http.NewRequest("GET", "/test/query/rewrite?x_test=foo&y_test=bar", nil) @@ -429,6 +479,52 @@ func TestRewriterTriggers(t *testing.T) { r, } }, + func() TestDef { + var jsonStr = []byte(`{"foo":"barxxx", "fooble":"baryyy"}`) + r, _ := http.NewRequest("POST", "/test/pl/rewrite", bytes.NewBuffer(jsonStr)) + + hOpt := apidef.StringRegexMap{MatchPattern: "bar\\w*"} + hOpt.Init() + + return TestDef{ + "Payload Multiple Match", + "/test/pl/rewrite", "/change/to/me/ignore", + "/test/pl/rewrite", "/change/to/me/barxxx/baryyy", + []apidef.RoutingTrigger{ + { + On: apidef.Any, + Options: apidef.RoutingTriggerOptions{ + PayloadMatches: hOpt, + }, + RewriteTo: "/change/to/me/$tyk_context.trigger-0-payload-0/$tyk_context.trigger-0-payload-1", + }, + }, + r, + } + }, + func() TestDef { + var jsonStr = []byte(`{"foo":"barxxx", "fooble":"baryyy"}`) + r, _ := http.NewRequest("POST", "/test/pl/rewrite", bytes.NewBuffer(jsonStr)) + + hOpt := apidef.StringRegexMap{MatchPattern: "bar(\\w*)"} + hOpt.Init() + + return TestDef{ + "Payload Multiple Match Groups", + "/test/pl/rewrite", "/change/to/me/ignore", + "/test/pl/rewrite", "/change/to/me/xxx/yyy", + []apidef.RoutingTrigger{ + { + On: apidef.Any, + Options: apidef.RoutingTriggerOptions{ + PayloadMatches: hOpt, + }, + RewriteTo: "/change/to/me/$tyk_context.trigger-0-payload-0-0/$tyk_context.trigger-0-payload-1-0", + }, + }, + r, + } + }, func() TestDef { r, _ := http.NewRequest("GET", "/test/foo/rewrite", nil) hOpt := apidef.StringRegexMap{MatchPattern: "foo"} @@ -452,6 +548,29 @@ func TestRewriterTriggers(t *testing.T) { r, } }, + func() TestDef { + r, _ := http.NewRequest("GET", "/test/foobar/rewrite", nil) + hOpt := apidef.StringRegexMap{MatchPattern: "foo(\\w+)"} + hOpt.Init() + + return TestDef{ + "PathPart Single Group", + "/test/foobar/rewrite", "/change/to/me/ignore", + "/test/foobar/rewrite", "/change/to/me/bar", + []apidef.RoutingTrigger{ + { + On: apidef.Any, + Options: apidef.RoutingTriggerOptions{ + PathPartMatches: map[string]apidef.StringRegexMap{ + "pathpart": hOpt, + }, + }, + RewriteTo: "/change/to/me/$tyk_context.trigger-0-pathpart-0-0", + }, + }, + r, + } + }, func() TestDef { r, _ := http.NewRequest("GET", "/test/foo/rewrite/foo", nil) hOpt := apidef.StringRegexMap{MatchPattern: "foo"} @@ -498,6 +617,29 @@ func TestRewriterTriggers(t *testing.T) { r, } }, + func() TestDef { + r, _ := http.NewRequest("GET", "/test/foo/rewrite", nil) + hOpt := apidef.StringRegexMap{MatchPattern: "bar-(\\w+)"} + hOpt.Init() + + return TestDef{ + "Meta Simple Group", + "/test/foo/rewrite", "/change/to/me/ignore", + "/test/foo/rewrite", "/change/to/me/baz", + []apidef.RoutingTrigger{ + { + On: apidef.Any, + Options: apidef.RoutingTriggerOptions{ + SessionMetaMatches: map[string]apidef.StringRegexMap{ + "rewrite": hOpt, + }, + }, + RewriteTo: "/change/to/me/$tyk_context.trigger-0-rewrite-0", + }, + }, + r, + } + }, } for _, tf := range tests { tc := tf() @@ -510,7 +652,7 @@ func TestRewriterTriggers(t *testing.T) { ctxSetSession(tc.req, &user.SessionState{ MetaData: map[string]interface{}{ - "rewrite": "bar", + "rewrite": "bar-baz", }, }, "", false) From 990474b4369a223c92c547d2fbc3d4d22fa58eee Mon Sep 17 00:00:00 2001 From: dskoval <44838975+dskoval@users.noreply.github.com> Date: Fri, 9 Nov 2018 13:41:23 +0200 Subject: [PATCH 2/2] Add caching logic for FindStringSubmatch in tyk/regexp --- regexp/cache_regexp_str_ret_slice_str.go | 45 ++++++++++++++++++++++++ regexp/regexp.go | 5 +-- regexp/regexp_test.go | 44 +++++++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 regexp/cache_regexp_str_ret_slice_str.go diff --git a/regexp/cache_regexp_str_ret_slice_str.go b/regexp/cache_regexp_str_ret_slice_str.go new file mode 100644 index 000000000000..9c3e9cc7889f --- /dev/null +++ b/regexp/cache_regexp_str_ret_slice_str.go @@ -0,0 +1,45 @@ +package regexp + +import ( + "regexp" + "time" +) + +type regexpStrRetSliceStrCache struct { + *cache +} + +func newRegexpStrRetSliceStrCache(ttl time.Duration, isEnabled bool) *regexpStrRetSliceStrCache { + return ®expStrRetSliceStrCache{ + cache: newCache( + ttl, + isEnabled, + ), + } +} + +func (c *regexpStrRetSliceStrCache) do(r *regexp.Regexp, s string, noCacheFn func(s string) []string) []string { + // return if cache is not enabled + if !c.enabled() { + return noCacheFn(s) + } + + // generate key, check key size + key := r.String() + s + if len(key) > maxKeySize { + return noCacheFn(s) + } + + // cache hit + if res, found := c.getStrSlice(key); found { + return res + } + + // cache miss, add to cache if value is not too big + res := noCacheFn(s) + if len(res) <= maxValueSize { + c.add(key, res) + } + + return res +} diff --git a/regexp/regexp.go b/regexp/regexp.go index 105c5d666671..bf3d0c1e5f2f 100644 --- a/regexp/regexp.go +++ b/regexp/regexp.go @@ -18,6 +18,7 @@ var ( replaceAllStringCache = newRegexpStrStrRetStrCache(defaultCacheItemTTL, true) replaceAllLiteralStringCache = newRegexpStrStrRetStrCache(defaultCacheItemTTL, true) replaceAllStringFuncCache = newRegexpStrFuncRetStrCache(defaultCacheItemTTL, true) + findStringSubmatchCache = newRegexpStrRetSliceStrCache(defaultCacheItemTTL, true) findAllStringCache = newRegexpStrIntRetSliceStrCache(defaultCacheItemTTL, true) findAllStringSubmatchCache = newRegexpStrIntRetSliceSliceStrCache(defaultCacheItemTTL, true) ) @@ -41,6 +42,7 @@ func ResetCache(ttl time.Duration, isEnabled bool) { replaceAllStringCache.reset(ttl, isEnabled) replaceAllLiteralStringCache.reset(ttl, isEnabled) replaceAllStringFuncCache.reset(ttl, isEnabled) + findStringSubmatchCache.reset(ttl, isEnabled) findAllStringCache.reset(ttl, isEnabled) findAllStringSubmatchCache.reset(ttl, isEnabled) } @@ -320,8 +322,7 @@ func (re *Regexp) FindStringSubmatch(s string) []string { if re.Regexp == nil { return []string{} } - // TODO: add cache for FindStringSubmatch - return re.Regexp.FindStringSubmatch(s) + return findStringSubmatchCache.do(re.Regexp, s, re.Regexp.FindStringSubmatch) } // FindStringSubmatchIndex is the same as regexp.Regexp.FindStringSubmatchIndex but returns cached result instead. diff --git a/regexp/regexp_test.go b/regexp/regexp_test.go index b6590210b3b8..df47dce10fc0 100644 --- a/regexp/regexp_test.go +++ b/regexp/regexp_test.go @@ -567,3 +567,47 @@ func BenchmarkRegexpFindAllStringSubmatch(b *testing.B) { b.Log(res) } + +func TestFindStringSubmatch(t *testing.T) { + ResetCache(defaultCacheItemTTL, true) + + // 1st miss + rx := MustCompile("abc(\\w)") + res := rx.FindStringSubmatch("qweabcxyzabc123abcz") + expectedRes := []string{"abcx", "x"} + + if !reflect.DeepEqual(res, expectedRes) { + t.Error("Expected :", expectedRes, " Got:", res) + } + // 2nd hit + res2 := rx.FindStringSubmatch("qweabcxyzabc123abcz") + if !reflect.DeepEqual(res2, expectedRes) { + t.Error("Expected :", expectedRes, " Got:", res2) + } +} + +func TestTestFindStringSubmatchRegexpNotSet(t *testing.T) { + ResetCache(defaultCacheItemTTL, true) + + rx := &Regexp{} + + res := rx.FindStringSubmatch("qweabcxyzabc123abcz") + if len(res) > 0 { + t.Error("Expected 0 length slice returned. Got:", res) + } +} + +func BenchmarkRegexpFindStringSubmatch(b *testing.B) { + b.ReportAllocs() + + ResetCache(defaultCacheItemTTL, true) + + rx := MustCompile("abc(\\w)") + var res []string + + for i := 0; i < b.N; i++ { + res = rx.FindStringSubmatch("qweabcxyzabc123abcz") + } + + b.Log(res) +}