OCPBUGS-27741: Properly handle route path while rewriting

Process the route's spec.path while using the
`haproxy.router.openshift.io/rewrite-target` annotation to prevent
HAProxy configuration issues.

Previously, values in the route's path would be interpreted as
regex meta characters. Now, this fix forces literal interpretation
of the characters in spec.path when matching for rewriting. This
change introduces some unavoidable incompatibilities if users were
previously using regex meta characters in a spec.path.

images/router/haproxy/conf/haproxy-config.template

@@ -683,9 +683,9 @@ backend {{ genBackendNamePrefix $cfg.TLSTermination }}:{{ $cfgIdx }}
         {{- with $pathRewriteTarget := firstMatch $pathRewriteTargetPattern (index $cfg.Annotations "haproxy.router.openshift.io/rewrite-target") }}
   # Path rewrite target
           {{- if eq $pathRewriteTarget "/" }}
-  http-request replace-path ^{{ $cfg.Path }}/?(.*)$ '{{ processRewriteTarget $pathRewriteTarget }}'
+  http-request replace-path '^\Q{{ processRewritePath $cfg.Path }}/\E?(.*)$' '{{ processRewriteTarget $pathRewriteTarget }}'
           {{- else }}
-  http-request replace-path ^{{ $cfg.Path }}(.*)$ '{{ processRewriteTarget $pathRewriteTarget }}'
+  http-request replace-path '^\Q{{ processRewritePath $cfg.Path }}\E(.*)$' '{{ processRewriteTarget $pathRewriteTarget }}'
           {{- end }}
         {{- end }}{{/* rewrite target */}}
 

pkg/router/template/template_helper.go

@@ -415,5 +415,6 @@ var helperFunctions = template.FuncMap{
 	"clipHAProxyTimeoutValue": clipHAProxyTimeoutValue, //clips extrodinarily high timeout values to be below the maximum allowed timeout value
 	"parseIPList":             parseIPList,             //parses the list of IPs/CIDRs (IPv4/IPv6)
 
-	"processRewriteTarget": rewritetarget.SanitizeInput, //sanitizes `haproxy.router.openshift.io/rewrite-target` annotation
+	"processRewriteTarget": rewritetarget.SanitizeRewriteTargetInput, //sanitizes `haproxy.router.openshift.io/rewrite-target` annotation
+	"processRewritePath":   rewritetarget.SanitizeRewritePathInput,   //sanitizes route path for rewrite-target functionality
 }

pkg/router/template/util/rewritetarget/rewritetarget.go

@@ -128,7 +128,7 @@ func processSingleQuotes(char rune, escaped bool) runeResult {
 	return runeResult{value: string(char), escaped: char == '\\' && !escaped}
 }
 
-// SanitizeInput processes the `haproxy.router.openshift.io/rewrite-target`
+// SanitizeRewriteTargetInput processes the `haproxy.router.openshift.io/rewrite-target`
 // annotation value for API compatibility while properly handling values
 // with spaces, backslashes, and other special characters. Because the
 // annotation value was initially introduced without being enclosed in
@@ -137,7 +137,7 @@ func processSingleQuotes(char rune, escaped bool) runeResult {
 // OCPBUGS-22739. However, we must still maintain API compatibility after
 // this change: the annotation values MUST be interpreted to the same values
 // after updating to enclose the value in single quotes.
-func SanitizeInput(val string) string {
+func SanitizeRewriteTargetInput(val string) string {
 	var encounteredCommentMarker bool
 
 	val = processRunes(val, newProcessHashCreator(&encounteredCommentMarker))
@@ -157,3 +157,29 @@ func SanitizeInput(val string) string {
 
 	return val
 }
+
+// escapeSingleQuotes escapes all single quotes regardless if
+// they appear in pairs. It assumes the argument is already wrapped
+// in ” so that the escape sequence '\” will need to be added.
+func escapeSingleQuotes(val string) string {
+	return strings.ReplaceAll(val, `'`, `'\''`)
+}
+
+// SanitizeRewritePathInput processes the route's spec.path to be used
+// in the <match-regex> argument of the `http-request replace-path <match-regex> <replace-fmt>`
+// HAProxy configuration. This configuration is enabled when the
+// `haproxy.router.openshift.io/rewrite-target` annotation is provided.
+// Previous to this fix, the route's spec.path input to the <match-regex>
+// argument wasn't interpreted literally, causing characters in spec.path
+// to be interpreted as a regex meta characters. The solution wraps the
+// <match-regex> argument single quotes ” and uses /Q and /E to force literal
+// regex interpretation. However, we still must escape single quotes ' in the
+// spec.path to avoid syntax errors. All other characters are interpreted literally.
+// The key difference in handling <match-regex> (spec.path) and <replace-fmt>
+// (rewrite annotation) compatibility is that it is impossible to ensure compatibility
+// with values that were previously interpreted as regex meta characters.
+func SanitizeRewritePathInput(val string) string {
+	val = escapeSingleQuotes(val)
+
+	return val
+}

pkg/router/template/util/rewritetarget/rewritetarget_test.go

@@ -5,7 +5,7 @@ import (
 	"testing"
 )
 
-func Test_SanitizeInput(t *testing.T) {
+func Test_SanitizeRewriteTargetInput(t *testing.T) {
 	testCases := []struct {
 		name   string
 		input  string
@@ -85,7 +85,35 @@ func Test_SanitizeInput(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			got := rewritetarget.SanitizeInput(tc.input)
+			got := rewritetarget.SanitizeRewriteTargetInput(tc.input)
+			if got != tc.output {
+				t.Errorf("Failure: expected %s, got %s", tc.output, got)
+			}
+		})
+	}
+}
+
+func Test_SanitizeRewritePathInput(t *testing.T) {
+	testCases := []struct {
+		name   string
+		input  string
+		output string
+	}{
+		{
+			name:   "single quotes should be always be escaped",
+			input:  `'foo'foo\'`,
+			output: `'\''foo'\''foo\'\''`,
+		},
+		{
+			name:   "nothing should change except for single quotes",
+			input:  `\\foo\"foo"\#foo\foo'foo\'#foo`,
+			output: `\\foo\"foo"\#foo\foo'\''foo\'\''#foo`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := rewritetarget.SanitizeRewritePathInput(tc.input)
 			if got != tc.output {
 				t.Errorf("Failure: expected %s, got %s", tc.output, got)
 			}