From 0475238c52ccb7cfc5aabf06bd42bac6f7c7e994 Mon Sep 17 00:00:00 2001
From: Philip Conrad <philipaconrad@gmail.com>
Date: Thu, 20 Oct 2022 03:09:31 -0400
Subject: [PATCH] topdown/glob: Fix deferred mutex unlock issue. (#5274)

This commit wraps up the mutex-requiring code within the `glob.match`
builtin in helper function, so that the deferred mutex unlock will
always occur before the call at the end of the function to `iter()`.
This helps prevent deadlocks during evaluation.

Fixes #5273.

Signed-off-by: Philip Conrad <philipaconrad@gmail.com>
---
 .../globmatch/test-globmatch-issue-5273.yaml  | 17 ++++++++++++++++
 topdown/glob.go                               | 20 +++++++++++++------
 2 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 test/cases/testdata/globmatch/test-globmatch-issue-5273.yaml

diff --git a/test/cases/testdata/globmatch/test-globmatch-issue-5273.yaml b/test/cases/testdata/globmatch/test-globmatch-issue-5273.yaml
new file mode 100644
index 0000000000..5962d994b8
--- /dev/null
+++ b/test/cases/testdata/globmatch/test-globmatch-issue-5273.yaml
@@ -0,0 +1,17 @@
+cases:
+- data: {}
+  modules:
+  - |
+    package generated
+
+    p[x] {
+      glob.match("*.github.com", ["."], "api.github.com", x)
+      glob.match("*.github.com", ["."], "api.github.com", x)
+    }
+  # See: https://github.com/open-policy-agent/opa/issues/5273
+  note: globmatch/no deadlocks for glob match
+  query: data.generated.p = x
+  sort_bindings: true
+  want_result:
+  - x:
+    - true
diff --git a/topdown/glob.go b/topdown/glob.go
index 5bdb7e2565..2afead73a2 100644
--- a/topdown/glob.go
+++ b/topdown/glob.go
@@ -18,6 +18,7 @@ func builtinGlobMatch(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Ter
 	if err != nil {
 		return err
 	}
+
 	var delimiters []rune
 	switch operands[1].Value.(type) {
 	case ast.Null:
@@ -33,8 +34,8 @@ func builtinGlobMatch(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Ter
 	default:
 		return builtins.NewOperandTypeErr(2, operands[1].Value, "array", "null")
 	}
-	match, err := builtins.StringOperand(operands[2].Value, 3)
 
+	match, err := builtins.StringOperand(operands[2].Value, 3)
 	if err != nil {
 		return err
 	}
@@ -47,19 +48,26 @@ func builtinGlobMatch(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Ter
 	}
 	id := builder.String()
 
+	m, err := globCompileAndMatch(id, string(pattern), string(match), delimiters)
+	if err != nil {
+		return err
+	}
+	return iter(ast.BooleanTerm(m))
+}
+
+func globCompileAndMatch(id, pattern, match string, delimiters []rune) (bool, error) {
 	globCacheLock.Lock()
 	defer globCacheLock.Unlock()
 	p, ok := globCache[id]
 	if !ok {
 		var err error
-		if p, err = glob.Compile(string(pattern), delimiters...); err != nil {
-			return err
+		if p, err = glob.Compile(pattern, delimiters...); err != nil {
+			return false, err
 		}
 		globCache[id] = p
 	}
-
-	m := p.Match(string(match))
-	return iter(ast.BooleanTerm(m))
+	out := p.Match(match)
+	return out, nil
 }
 
 func builtinGlobQuoteMeta(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) error) error {