From 67955d5fba20221d628636fa2e71a9ad074af3ec Mon Sep 17 00:00:00 2001
From: Dibyo Mukherjee <dibyo@google.com>
Date: Wed, 22 Jan 2020 15:54:38 -0500
Subject: [PATCH] Allow brackets inside Tekton expressions

We were previously using a regex to extract expressions within
TriggerBindings. The regex only extracted between the initial `$(`
and the first occurrence of the `)`. This meant that some valid
JSONPath expressions that contained brackets (e.g. filters) were
incorrectly extracted.

This commit fixes the issue by splitting the string on `$(` and then
extracting until the first "unbalanced" occurrence of `)`.

Fixes #365

Co-Authored-By: Andrea Frittoli <andrea.frittoli@uk.ibm.com>
Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
---
 pkg/template/event.go         |  2 +-
 pkg/template/event_test.go    | 10 ++++++
 pkg/template/jsonpath.go      | 34 ++++++++++++++++++++
 pkg/template/jsonpath_test.go | 59 +++++++++++++++++++++++++++++++++++
 4 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/pkg/template/event.go b/pkg/template/event.go
index ea00b69742..1d6045a957 100644
--- a/pkg/template/event.go
+++ b/pkg/template/event.go
@@ -88,7 +88,7 @@ func applyEventValuesToParams(params []pipelinev1.Param, body []byte, header htt
 	for idx, p := range params {
 		pValue := p.Value.StringVal
 		// Find all expressions wrapped in $() from the value
-		expressions := tektonVar.FindAllString(pValue, -1)
+		expressions := findTektonExpressions(pValue)
 		for _, expr := range expressions {
 			val, err := ParseJSONPath(event, expr)
 			if err != nil {
diff --git a/pkg/template/event_test.go b/pkg/template/event_test.go
index fd0674824e..257458698a 100644
--- a/pkg/template/event_test.go
+++ b/pkg/template/event_test.go
@@ -150,6 +150,16 @@ func TestApplyEventValuesToParams(t *testing.T) {
 			bldr.Param("foo", `val1`),
 			bldr.Param("bar", `val2`),
 		},
+	}, {
+		name:   "Array filters",
+		body:   json.RawMessage(`{"child":[{"a": "b", "w": "1"}, {"a": "c", "w": "2"}, {"a": "d", "w": "3"}]}`),
+		params: []pipelinev1.Param{bldr.Param("a", "$(body.child[?(@.a == 'd')].w)")},
+		want:   []pipelinev1.Param{bldr.Param("a", "3")},
+	}, {
+		name:   "filters + multiple JSONPath expressions",
+		body:   json.RawMessage(`{"child":[{"a": "b", "w": "1"}, {"a": "c", "w": "2"}, {"a": "d", "w": "3"}]}`),
+		params: []pipelinev1.Param{bldr.Param("a", "$(body.child[?(@.a == 'd')].w) : $(body.child[0].a)")},
+		want:   []pipelinev1.Param{bldr.Param("a", "3 : b")},
 	}}
 
 	for _, tt := range tests {
diff --git a/pkg/template/jsonpath.go b/pkg/template/jsonpath.go
index beb1a80858..2af291589d 100644
--- a/pkg/template/jsonpath.go
+++ b/pkg/template/jsonpath.go
@@ -146,3 +146,37 @@ func relaxedJSONPathExpression(pathExpression string) (string, error) {
 func isTektonExpr(expr string) bool {
 	return tektonVar.MatchString(expr)
 }
+
+// findTektonExpressions searches for and returns a slice of
+// all substrings that are wrapped in $()
+func findTektonExpressions(in string) []string {
+	results := []string{}
+
+	// No expressions to return
+	if !strings.Contains(in, "$(") {
+		return results
+	}
+	// Splits string on $( to find potential Tekton expressions
+	maybeExpressions := strings.Split(in, "$(")
+	for _, e := range maybeExpressions[1:] { // Split always returns at least one element
+		// Iterate until we find the first unbalanced )
+		numOpenBrackets := 0
+		for i, ch := range e {
+			switch ch {
+			case '(':
+				numOpenBrackets++
+			case ')':
+				numOpenBrackets--
+				if numOpenBrackets < 0 {
+					results = append(results, fmt.Sprintf("$(%s)", e[:i]))
+				}
+			default:
+				continue
+			}
+			if numOpenBrackets < 0 {
+				break
+			}
+		}
+	}
+	return results
+}
diff --git a/pkg/template/jsonpath_test.go b/pkg/template/jsonpath_test.go
index 2018bfbbbe..e1d7a5b9d0 100644
--- a/pkg/template/jsonpath_test.go
+++ b/pkg/template/jsonpath_test.go
@@ -83,6 +83,11 @@ func TestParseJSONPath(t *testing.T) {
 		in:   `{"body":["", null, "thing"]}`,
 		expr: "$(body[:2])",
 		want: `["", null]`,
+	}, {
+		name: "Array filters",
+		in:   `{"body":{"child":[{"a": "b", "w": "1"}, {"a": "c", "w": "2"}, {"a": "d", "w": "3"}]}}`,
+		expr: "$(body.child[?(@.a == 'd')].w)",
+		want: "3",
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -213,3 +218,57 @@ func TestRelaxedJSONPathExpression_Error(t *testing.T) {
 		})
 	}
 }
+
+func TestFindTektonExpressions(t *testing.T) {
+	tcs := []struct {
+		in   string
+		want []string
+	}{{
+		in:   "$(body.blah)",
+		want: []string{"$(body.blah)"},
+	}, {
+		in:   "$(body.blah)-$(header.*)",
+		want: []string{"$(body.blah)", "$(header.*)"},
+	}, {
+		in:   "start:$(body.blah)//middle//$(header.one)-end",
+		want: []string{"$(body.blah)", "$(header.one)"},
+	}, {
+		in:   "start:$(body.[?(@.a == 'd')])-$(body.another-one)",
+		want: []string{"$(body.[?(@.a == 'd')])", "$(body.another-one)"},
+	}, {
+		in:   "$(this)-$(not-this",
+		want: []string{"$(this)"},
+	}, {
+		in:   "$body.)",
+		want: []string{},
+	}, {
+		in:   "($(body.blah))-and-$(body.foo)",
+		want: []string{"$(body.blah)", "$(body.foo)"},
+	}, {
+		in:   "(staticvalue)$(body.blah)",
+		want: []string{"$(body.blah)"},
+	}, {
+		in:   "asd)$(asd",
+		want: []string{},
+	}, {
+		in:   "onlystatic",
+		want: []string{},
+	}, {
+		in: "",
+		want: []string{},
+	}, {
+		in: "$())))",
+		want: []string{"$()"},
+	}, {
+		in: "$($())",
+		want: []string{"$()"},
+	}}
+
+	for _, tc := range tcs {
+		t.Run(tc.in, func(t *testing.T) {
+			if diff := cmp.Diff(tc.want, findTektonExpressions(tc.in)); diff != "" {
+				t.Fatalf("error -want/+got: %s", diff)
+			}
+		})
+	}
+}