Fix url rewrite issue #1989 (#1993)

- if varible not found, replace only varible with empty string - if sessin not found, replace session vars with empty string fix #1989
TykTechnologies · Nov 30, 2018 · 7eb8477 · 7eb8477
1 parent a5ddc52
commit 7eb8477
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 39 deletions.
diff --git a/mw_url_rewrite.go b/mw_url_rewrite.go
@@ -177,49 +177,38 @@ func urlRewrite(meta *apidef.URLRewriteMeta, r *http.Request) (string, error) {
 func replaceTykVariables(r *http.Request, in string, escape bool) string {
 	if strings.Contains(in, contextLabel) {
 		contextData := ctxGetData(r)
-
-		replaceGroups := contextMatch.FindAllStringSubmatch(in, -1)
-		for _, v := range replaceGroups {
-			contextKey := strings.Replace(v[0], contextLabel, "", 1)
-
-			if val, ok := contextData[contextKey]; ok {
-				valStr := valToStr(val)
-				// If contains url with domain
-				if escape && !strings.HasPrefix(valStr, "http") {
-					valStr = url.QueryEscape(valStr)
-				}
-				in = strings.Replace(in, v[0], valStr, -1)
-			} else {
-				in = ""
-			}
-		}
+		vars := contextMatch.FindAllString(in, -1)
+		in = replaceVariables(in, vars, contextData, contextLabel, escape)
 	}
 
 	if strings.Contains(in, metaLabel) {
-		// Meta data from the token
+		vars := metaMatch.FindAllString(in, -1)
 		session := ctxGetSession(r)
 		if session == nil {
-			return in
+			in = replaceVariables(in, vars, nil, metaLabel, escape)
+		} else {
+			in = replaceVariables(in, vars, session.MetaData, metaLabel, escape)
 		}
+	}
 
-		replaceGroups := metaMatch.FindAllStringSubmatch(in, -1)
-		for _, v := range replaceGroups {
-			contextKey := strings.Replace(v[0], metaLabel, "", 1)
-
-			val, ok := session.MetaData[contextKey]
-			if ok {
-				valStr := valToStr(val)
-				// If contains url with domain
-				if escape && !strings.HasPrefix(valStr, "http") {
-					valStr = url.QueryEscape(valStr)
-				}
-				in = strings.Replace(in, v[0], valStr, -1)
-			} else {
-				in = ""
+	return in
+}
+
+func replaceVariables(in string, vars []string, vals map[string]interface{}, label string, escape bool) string {
+	for _, v := range vars {
+		key := strings.Replace(v, label, "", 1)
+		val, ok := vals[key]
+		if ok {
+			valStr := valToStr(val)
+			// If contains url with domain
+			if escape && !strings.HasPrefix(valStr, "http") {
+				valStr = url.QueryEscape(valStr)
 			}
+			in = strings.Replace(in, v, valStr, -1)
+		} else {
+			in = strings.Replace(in, v, "", -1)
 		}
 	}
-
 	return in
 }
 

diff --git a/mw_url_rewrite_test.go b/mw_url_rewrite_test.go
@@ -599,6 +599,12 @@ func TestRewriterTriggers(t *testing.T) {
 			hOpt := apidef.StringRegexMap{MatchPattern: "bar"}
 			hOpt.Init()
 
+			ctxSetSession(r, &user.SessionState{
+				MetaData: map[string]interface{}{
+					"rewrite": "bar-baz",
+				},
+			}, "", false)
+
 			return TestDef{
 				"Meta Simple",
 				"/test/foo/rewrite", "/change/to/me/ignore",
@@ -622,6 +628,12 @@ func TestRewriterTriggers(t *testing.T) {
 			hOpt := apidef.StringRegexMap{MatchPattern: "bar-(\\w+)"}
 			hOpt.Init()
 
+			ctxSetSession(r, &user.SessionState{
+				MetaData: map[string]interface{}{
+					"rewrite": "bar-baz",
+				},
+			}, "", false)
+
 			return TestDef{
 				"Meta Simple Group",
 				"/test/foo/rewrite", "/change/to/me/ignore",
@@ -640,6 +652,88 @@ func TestRewriterTriggers(t *testing.T) {
 				r,
 			}
 		},
+		func() TestDef {
+			r, _ := http.NewRequest("GET", "/test/foo/rewrite", nil)
+			hOpt := apidef.StringRegexMap{MatchPattern: "bar"}
+			hOpt.Init()
+
+			ctxSetSession(r, &user.SessionState{
+				MetaData: map[string]interface{}{
+					"rewrite": "bar-baz",
+					"somevar": "someval",
+				},
+			}, "", false)
+
+			return TestDef{
+				"Meta Value from Session",
+				"/test/foo/rewrite", "/change/to/me/ignore",
+				"/test/foo/rewrite", "/change/to/me/bar/someval",
+				[]apidef.RoutingTrigger{
+					{
+						On: apidef.Any,
+						Options: apidef.RoutingTriggerOptions{
+							SessionMetaMatches: map[string]apidef.StringRegexMap{
+								"rewrite": hOpt,
+							},
+						},
+						RewriteTo: "/change/to/me/$tyk_context.trigger-0-rewrite/$tyk_meta.somevar",
+					},
+				},
+				r,
+			}
+		},
+		func() TestDef {
+			r, _ := http.NewRequest("GET", "/test/foo/rewrite", nil)
+			hOpt := apidef.StringRegexMap{MatchPattern: "foo"}
+			hOpt.Init()
+
+			ctxSetSession(r, &user.SessionState{
+				MetaData: map[string]interface{}{
+					"rewrite": "bar-baz",
+				},
+			}, "", false)
+
+			return TestDef{
+				"Variable not found",
+				"/test/foo/rewrite", "/change/to/me/ignore",
+				"/test/foo/rewrite", "/change/to/me/foo//",
+				[]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/$tyk_context.nonexistent/$tyk_meta.nonexistent",
+					},
+				},
+				r,
+			}
+		},
+		func() TestDef {
+			r, _ := http.NewRequest("GET", "/test/foo/rewrite", nil)
+			hOpt := apidef.StringRegexMap{MatchPattern: "foo"}
+			hOpt.Init()
+
+			return TestDef{
+				"Session not found",
+				"/test/foo/rewrite", "/change/to/me/ignore",
+				"/test/foo/rewrite", "/change/to/me/foo/",
+				[]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/$tyk_meta.nonexistent",
+					},
+				},
+				r,
+			}
+		},
 	}
 	for _, tf := range tests {
 		tc := tf()
@@ -650,12 +744,6 @@ func TestRewriterTriggers(t *testing.T) {
 				Triggers:     tc.triggerConf,
 			}
 
-			ctxSetSession(tc.req, &user.SessionState{
-				MetaData: map[string]interface{}{
-					"rewrite": "bar-baz",
-				},
-			}, "", false)
-
 			got, err := urlRewrite(&testConf, tc.req)
 			if err != nil {
 				t.Error("compile failed:", err)