From 9ef81530df7b8ac617f076ca5001a82c4a4100f9 Mon Sep 17 00:00:00 2001
From: Zander Hill <zander@xargs.io>
Date: Fri, 5 Jul 2024 01:57:41 -0700
Subject: [PATCH] (bug) Fix params quoting when using quotes on value (#602)

* (bug) Fix params quoting when using quotes on value

Fixes issue with yaml key `params` where it results in different
behavior than if the key was passed through env key.

Takes an approach that does not require backwards
incompatible changes nor does it require changing
the schema of yaml files.

------

**Expectation**: using params or env vars would both inject environmental variables into any subprocesses executed in the DAG.

**Actual**: they both inject env vars but differ in how they treat quotes.

Test dag:
```yaml
params: TEST_PARAM="something" TEST_PARAM2="SOMETHING ELSE"
env:
- ENV_PARAM: "something"
steps:
  - name: step1
    command: bash $HOME/src/dagu/tester.sh
```

```

printf "TEST_PARAM is %s | TEST_PARAM2 is %s\n" "$TEST_PARAM" "$TEST_PARAM2"

echo $TEST_PARAM/and/path
echo $ENV_PARAM/and/path
```

Results in output:
```
TEST_PARAM is "something" | TEST_PARAM2 is "SOMETHING ELSE"
"something"/and/path
something/and/path
```

Which is interpreted subtly differently in the inner script and caused me to debug and workaround it for a dag.

What I expect as output is:
```
TEST_PARAM is something | TEST_PARAM2 is SOMETHING ELSE
something/and/path
something/and/path
```

Note the difference for TEST_PARAM and ENV_PARAM. We can't leave off the quote marks for params, otherwise the arguments are parsed incorrectly by splitting on whitespace.

Because some users could rely on this behavior, one solution is to enable `params` key to take more structured data, like `env:` key.

Like this:
```
params:
- TEST_PARAM: "something"
- TEST_PARAM2: "SOMETHING ELSE"
env:
- ENV_PARAM: "something"
steps:
  - name: step1
    command: bash $HOME/src/dagu/tester.sh
```

* Adjust tests to reflect there's no forced quoting
---
 internal/dag/builder.go      |  7 ++-----
 internal/dag/builder_test.go | 12 ++++++------
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/internal/dag/builder.go b/internal/dag/builder.go
index ebce552a..47975239 100644
--- a/internal/dag/builder.go
+++ b/internal/dag/builder.go
@@ -426,13 +426,10 @@ func parseParams(
 
 // stringifyParam converts a paramPair to a string representation.
 func stringifyParam(param paramPair) string {
-	escapedValue := strings.ReplaceAll(param.value, `"`, `\"`)
-	quotedValue := fmt.Sprintf(`"%s"`, escapedValue)
-
 	if param.name != "" {
-		return fmt.Sprintf("%s=%s", param.name, quotedValue)
+		return fmt.Sprintf("%s=%s", param.name, param.value)
 	}
-	return quotedValue
+	return param.value
 }
 
 // processParams parses and processes the parameters for the DAG.
diff --git a/internal/dag/builder_test.go b/internal/dag/builder_test.go
index 784df0e6..94fa8fa7 100644
--- a/internal/dag/builder_test.go
+++ b/internal/dag/builder_test.go
@@ -156,12 +156,12 @@ func TestBuilder_BuildParams(t *testing.T) {
 				"Y":  "foo",
 				"Z":  "A B C",
 				"1":  "first",
-				"2":  `P1="foo"`,
-				"3":  `P2="BAR"`,
-				"4":  `P3="BAR"`,
-				"5":  `X="bar"`,
-				"6":  `Y="foo"`,
-				"7":  `Z="A B C"`,
+				"2":  `P1=foo`,
+				"3":  `P2=BAR`,
+				"4":  `P3=BAR`,
+				"5":  `X=bar`,
+				"6":  `Y=foo`,
+				"7":  `Z=A B C`,
 			},
 		},
 	}