From 3c1e69a06449b9f92b52350031230358cb354c78 Mon Sep 17 00:00:00 2001
From: stikkireddy <54602805+stikkireddy@users.noreply.github.com>
Date: Fri, 23 Jun 2023 06:56:54 -0400
Subject: [PATCH] Update variable regex to support hyphens (#503)

## Changes

Modified interpolation logic to use:
`\$\{([a-zA-Z]+([-_]*[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]*[a-zA-Z0-9]+)*)*)\}`

**Edit**: Suggested by @pietern
`\$\{([a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*)*)\}`
to be more selective and not allow consequent hyphens or underscores to
make the keys more readable.

Explanation:
1. All interpolation starts with `${` and ends with `}`
2. All interpolated locations are split by by `.`
3. All sections are expected to start with a alphabet `[a-zA-Z]`; no
numbers, hyphens or underscores.
4. All sections are expected to end with an alphanumeric `[a-zA-Z0-9]`
no hyphens or underscores

This change allows the current interpolation to be more permissive.

**Note** it does break backwards compatibility because `[a-zA-Z] !=
[\w]`. `\w` includes alphanumeric and underscores. `\w = [a-zA-Z0-9_]`

## Tests
There are two tests with examples of valid and invalid interpolation and
a test to validate expansion.
---
 bundle/config/interpolation/interpolation.go  |  3 +-
 .../interpolation/interpolation_test.go       | 55 +++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/bundle/config/interpolation/interpolation.go b/bundle/config/interpolation/interpolation.go
index e4ff0a7374..bf19804a05 100644
--- a/bundle/config/interpolation/interpolation.go
+++ b/bundle/config/interpolation/interpolation.go
@@ -17,7 +17,8 @@ import (
 
 const Delimiter = "."
 
-var re = regexp.MustCompile(`\$\{(\w+(\.\w+)*)\}`)
+// must start with alphabet, support hyphens and underscores in middle but must end with character
+var re = regexp.MustCompile(`\$\{([a-zA-Z]+([-_]?[a-zA-Z0-9]+)*(\.[a-zA-Z]+([-_]?[a-zA-Z0-9]+)*)*)\}`)
 
 type stringField struct {
 	path string
diff --git a/bundle/config/interpolation/interpolation_test.go b/bundle/config/interpolation/interpolation_test.go
index c444af2e4e..83254c9b0b 100644
--- a/bundle/config/interpolation/interpolation_test.go
+++ b/bundle/config/interpolation/interpolation_test.go
@@ -51,6 +51,61 @@ func TestInterpolationVariables(t *testing.T) {
 	assert.Equal(t, "a", f.C)
 }
 
+func TestInterpolationVariablesSpecialChars(t *testing.T) {
+	type bar struct {
+		A string `json:"a-b"`
+		B string `json:"b_c"`
+		C string `json:"c-_a"`
+	}
+	f := bar{
+		A: "a",
+		B: "${a-b}",
+		C: "${a-b}",
+	}
+
+	err := expand(&f)
+	require.NoError(t, err)
+
+	assert.Equal(t, "a", f.A)
+	assert.Equal(t, "a", f.B)
+	assert.Equal(t, "a", f.C)
+}
+
+func TestInterpolationValidMatches(t *testing.T) {
+	expectedMatches := map[string]string{
+		"${hello_world.world_world}": "hello_world.world_world",
+		"${helloworld.world-world}":  "helloworld.world-world",
+		"${hello-world.world-world}": "hello-world.world-world",
+	}
+	for interpolationStr, expectedMatch := range expectedMatches {
+		match := re.FindStringSubmatch(interpolationStr)
+		assert.True(t, len(match) > 0,
+			"Failed to match %s and find %s", interpolationStr, expectedMatch)
+		assert.Equal(t, expectedMatch, match[1],
+			"Failed to match the exact pattern %s and find %s", interpolationStr, expectedMatch)
+	}
+}
+
+func TestInterpolationInvalidMatches(t *testing.T) {
+	invalidMatches := []string{
+		"${hello_world-.world_world}",   // the first segment ending must not end with hyphen (-)
+		"${hello_world-_.world_world}",  // the first segment ending must not end with underscore (_)
+		"${helloworld.world-world-}",    // second segment must not end with hyphen (-)
+		"${helloworld-.world-world}",    // first segment must not end with hyphen (-)
+		"${helloworld.-world-world}",    // second segment must not start with hyphen (-)
+		"${-hello-world.-world-world-}", // must not start or end with hyphen (-)
+		"${_-_._-_.id}",                 // cannot use _- in sequence
+		"${0helloworld.world-world}",    // interpolated first section shouldn't start with number
+		"${helloworld.9world-world}",    // interpolated second section shouldn't start with number
+		"${a-a.a-_a-a.id}",              // fails because of -_ in the second segment
+		"${a-a.a--a-a.id}",              // fails because of -- in the second segment
+	}
+	for _, invalidMatch := range invalidMatches {
+		match := re.FindStringSubmatch(invalidMatch)
+		assert.True(t, len(match) == 0, "Should be invalid interpolation: %s", invalidMatch)
+	}
+}
+
 func TestInterpolationWithPointers(t *testing.T) {
 	fd := "${a}"
 	f := foo{