context: put quotes around flagvalue incase of whitespaced values (#636)

Resolves #541

While running the sync command it starts to generate cp commands in the
generateCommand function and writes it to a pipe where it is read by the
Run function of the Run command. In the Run function, there is a line

`fields, err := shellquote.Split(line)`

to split the generated cp command into its fields.



 For a command



 
```
 
s5cmd sync --cache-control ‘public, max-age=31536000, immutable’ /Users/ataberk/desktop/test s3://s5cmd-test2
```

We would have expected the generateCommand function to write into the
pipe a command



```
cp --cache-control='public, max-age=31536000, immutable' --raw='true' \"/Users/ataberk/desktop/test/hello.txt\" \"s3://s5cmd-test2/test/hello.txt\"
```

But instead, it writes the command



```
cp --cache-control=public, max-age=31536000, immutable --raw=true \"/Users/ataberk/desktop/test/hello.txt\" \"s3://s5cmd-test2/test/hello.txt\"
```

![Screenshot 2023-08-05 at 13 45
52](https://github.com/peak/s5cmd/assets/30214288/da4db89a-49c2-41ca-9020-89d12484e72c)

This causes the `shellquote.Split(line)` to split fields incorrectly.
This causes `flagset.Parse(fields)` to populate flagset incorrectly and
as a result we end up with an error.

![Screenshot 2023-08-05 at 05 53
43](https://github.com/peak/s5cmd/assets/30214288/6777a474-a220-4b95-b158-16e2848402b5)

The main problem occurs in the generateCommand function while appending
flags without quotes. If we add quotes around the flagValue while
generating a command. The `shellquote.Split(line)` acts as intended and
successfully sync with the given cache-control metadata in S3

![Screenshot 2023-08-05 at 13 58
28](https://github.com/peak/s5cmd/assets/30214288/534aaa13-31e9-46bc-959f-85506d66917d)

![Screenshot 2023-08-05 at 13 59
31](https://github.com/peak/s5cmd/assets/30214288/64817fa7-7467-4046-85d3-f3e694426f63)

Co-authored-by: Ataberk Gürel <ataberk@Ataberks-Mini.home>

CHANGELOG.md

@@ -15,6 +15,7 @@
 - Upgraded minimum required Go version to 1.19. ([#583](https://github.com/peak/s5cmd/pull/583))
 
 #### Bugfixes
+- Fixed a bug that causes `sync` command with whitespaced flag value to fail. ([#541](https://github.com/peak/s5cmd/issues/541))
 - Fixed a bug introduced with `external sort` support in `sync` command which prevents `sync` to an empty destination with `--delete` option. ([#576](https://github.com/peak/s5cmd/issues/576))
 - Fixed a bug in `sync` command, which previously caused the command to continue running even if an error was received from the destination bucket. ([#564](https://github.com/peak/s5cmd/issues/564))
 - Fixed a bug that causes local files to be lost if downloads fail. ([#479](https://github.com/peak/s5cmd/issues/479))

command/context.go

@@ -73,7 +73,7 @@ func generateCommand(c *cli.Context, cmd string, defaultFlags map[string]interfa
 
 	flags := []string{}
 	for flagname, flagvalue := range defaultFlags {
-		flags = append(flags, fmt.Sprintf("--%s=%v", flagname, flagvalue))
+		flags = append(flags, fmt.Sprintf("--%s='%v'", flagname, flagvalue))
 	}
 
 	isDefaultFlag := func(flagname string) bool {
@@ -88,7 +88,7 @@ func generateCommand(c *cli.Context, cmd string, defaultFlags map[string]interfa
 		}
 
 		for _, flagvalue := range contextValue(c, flagname) {
-			flags = append(flags, fmt.Sprintf("--%s=%s", flagname, flagvalue))
+			flags = append(flags, fmt.Sprintf("--%s='%s'", flagname, flagvalue))
 		}
 	}
 

command/context_test.go

@@ -2,7 +2,6 @@ package command
 
 import (
 	"flag"
-	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -46,7 +45,25 @@ func TestGenerateCommand(t *testing.T) {
 				mustNewURL(t, "s3://bucket/key1"),
 				mustNewURL(t, "s3://bucket/key2"),
 			},
-			expectedCommand: `cp --acl=public-read --raw=true "s3://bucket/key1" "s3://bucket/key2"`,
+			expectedCommand: `cp --acl='public-read' --raw='true' "s3://bucket/key1" "s3://bucket/key2"`,
+		},
+		{
+			name: "cli-flag-with-whitespaced-flag-value",
+			cmd:  "cp",
+			flags: []cli.Flag{
+				&cli.StringFlag{
+					Name:  "cache-control",
+					Value: "public, max-age=31536000, immutable",
+				},
+			},
+			defaultFlags: map[string]interface{}{
+				"raw": true,
+			},
+			urls: []*url.URL{
+				mustNewURL(t, "s3://bucket/key1"),
+				mustNewURL(t, "s3://bucket/key2"),
+			},
+			expectedCommand: `cp --cache-control='public, max-age=31536000, immutable' --raw='true' "s3://bucket/key1" "s3://bucket/key2"`,
 		},
 		{
 			name: "same-flag-should-be-ignored-if-given-from-both-default-and-cli-flags",
@@ -64,7 +81,7 @@ func TestGenerateCommand(t *testing.T) {
 				mustNewURL(t, "s3://bucket/key1"),
 				mustNewURL(t, "s3://bucket/key2"),
 			},
-			expectedCommand: `cp --raw=true "s3://bucket/key1" "s3://bucket/key2"`,
+			expectedCommand: `cp --raw='true' "s3://bucket/key1" "s3://bucket/key2"`,
 		},
 		{
 			name: "ignore-non-shared-flag",
@@ -101,7 +118,7 @@ func TestGenerateCommand(t *testing.T) {
 				mustNewURL(t, "s3://bucket/key1"),
 				mustNewURL(t, "s3://bucket/key2"),
 			},
-			expectedCommand: `cp --concurrency=6 --flatten=true --force-glacier-transfer=true --raw=true "s3://bucket/key1" "s3://bucket/key2"`,
+			expectedCommand: `cp --concurrency='6' --flatten='true' --force-glacier-transfer='true' --raw='true' "s3://bucket/key1" "s3://bucket/key2"`,
 		},
 		{
 			name: "string-slice-flag",
@@ -116,7 +133,7 @@ func TestGenerateCommand(t *testing.T) {
 				mustNewURL(t, "/source/dir"),
 				mustNewURL(t, "s3://bucket/prefix/"),
 			},
-			expectedCommand: `cp --exclude=*.log --exclude=*.txt "/source/dir" "s3://bucket/prefix/"`,
+			expectedCommand: `cp --exclude='*.log' --exclude='*.txt' "/source/dir" "s3://bucket/prefix/"`,
 		},
 		{
 			name:  "command-with-multiple-args",
@@ -155,10 +172,12 @@ func TestGenerateCommand(t *testing.T) {
 			// and methods to update context are package-private, so write simple
 			// flag parser to update context value.
 			set.VisitAll(func(f *flag.Flag) {
-				value := strings.Trim(f.Value.String(), "[")
-				value = strings.Trim(value, "]")
-				for _, v := range strings.Fields(value) {
-					ctx.Set(f.Name, v)
+				if v, ok := f.Value.(*cli.StringSlice); ok {
+					for _, s := range v.Value() {
+						ctx.Set(f.Name, s)
+					}
+				} else {
+					ctx.Set(f.Name, f.Value.String())
 				}
 			})