From caf4867d356f7bbf1a28ac68800d3d54855afec9 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@suse.com>
Date: Wed, 14 Nov 2018 16:56:49 +0100
Subject: [PATCH] ProcessWords: preserve quotes

Preserve single and double quotes in ProcessWords to match the
function description and user intention.  Also add some simple
unit tests to catch potential future regressions.

Fixes: #108
Signed-off-by: Valentin Rothberg <vrothberg@suse.com>
---
 builder_test.go      | 17 ++++++------
 shell_parser.go      |  4 +--
 shell_parser_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 10 deletions(-)
 create mode 100644 shell_parser_test.go

diff --git a/builder_test.go b/builder_test.go
index 4626a205..2a53778e 100644
--- a/builder_test.go
+++ b/builder_test.go
@@ -222,7 +222,7 @@ func TestBuilder(t *testing.T) {
 			Dockerfile: "dockerclient/testdata/Dockerfile.env",
 			From:       "busybox",
 			Config: docker.Config{
-				Env:   []string{"name=value", "name2=value2a            value2b", "name1=value1", "name3=value3a\\n\"value3b\"", "name4=value4a\\\\nvalue4b"},
+				Env:   []string{"name=value", "name2=\"value2a            value2b\"", "name1=value1", "name3=\"value3a\\n\"value3b\"\"", "name4=\"value4a\\\\nvalue4b\""},
 				Image: "busybox",
 			},
 		},
@@ -245,7 +245,7 @@ func TestBuilder(t *testing.T) {
 			Config: docker.Config{
 				User:         "docker:root",
 				ExposedPorts: map[docker.Port]struct{}{"6000/tcp": {}, "3000/tcp": {}, "9000/tcp": {}, "5000/tcp": {}},
-				Env:          []string{"SCUBA=1 DUBA 3"},
+				Env:          []string{"SCUBA=\"1 DUBA 3\""},
 				Cmd:          []string{"/bin/sh", "-c", "echo 'test' | wc -"},
 				Image:        "busybox",
 				Volumes:      map[string]struct{}{"/test2": {}, "/test3": {}, "/test": {}},
@@ -327,8 +327,8 @@ func TestBuilder(t *testing.T) {
 				},
 			},
 			Config: docker.Config{
-				Env:    []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "FOO=value"},
-				Labels: map[string]string{"test": "value"},
+				Env:    []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", "FOO=\"value\""},
+				Labels: map[string]string{"test": "\"\"value\"\""},
 			},
 		},
 		{
@@ -337,8 +337,8 @@ func TestBuilder(t *testing.T) {
 			From:       "busybox",
 			Config: docker.Config{
 				Image:  "busybox",
-				Env:    []string{"FOO=value", "TEST=", "BAZ=first"},
-				Labels: map[string]string{"test": "value"},
+				Env:    []string{"FOO=\"value\"", "TEST=", "BAZ=first"},
+				Labels: map[string]string{"test": "\"\"value\"\""},
 			},
 			Runs: []Run{
 				{Shell: true, Args: []string{"echo $BAR"}},
@@ -473,8 +473,9 @@ func TestBuilder(t *testing.T) {
 			}
 			lastConfig := b.RunConfig
 			if !reflect.DeepEqual(test.Config, lastConfig) {
-				data, _ := json.Marshal(lastConfig)
-				t.Errorf("%d: unexpected config: %s", i, string(data))
+				lastData, _ := json.Marshal(lastConfig)
+				testData, _ := json.Marshal(test.Config)
+				t.Errorf("%d: unexpected config:\n%s\n%s", i, string(lastData), string(testData))
 			}
 		})
 	}
diff --git a/shell_parser.go b/shell_parser.go
index 65f1db6d..e89a7434 100644
--- a/shell_parser.go
+++ b/shell_parser.go
@@ -174,7 +174,7 @@ func (sw *shellWord) processSingleQuote() (string, error) {
 		result += string(ch)
 	}
 
-	return result, nil
+	return fmt.Sprintf("'%s'", result), nil
 }
 
 func (sw *shellWord) processDoubleQuote() (string, error) {
@@ -215,7 +215,7 @@ func (sw *shellWord) processDoubleQuote() (string, error) {
 		}
 	}
 
-	return result, nil
+	return fmt.Sprintf("\"%s\"", result), nil
 }
 
 func (sw *shellWord) processDollar() (string, error) {
diff --git a/shell_parser_test.go b/shell_parser_test.go
new file mode 100644
index 00000000..0d5a0b92
--- /dev/null
+++ b/shell_parser_test.go
@@ -0,0 +1,63 @@
+package imagebuilder
+
+import (
+	"testing"
+)
+
+func TestGetEnv(t *testing.T) {
+	sw := &shellWord{envs: nil}
+
+	sw.envs = []string{}
+	if sw.getEnv("foo") != "" {
+		t.Fatal("2 - 'foo' should map to ''")
+	}
+
+	sw.envs = []string{"foo"}
+	if sw.getEnv("foo") != "" {
+		t.Fatal("3 - 'foo' should map to ''")
+	}
+
+	sw.envs = []string{"foo="}
+	if sw.getEnv("foo") != "" {
+		t.Fatal("4 - 'foo' should map to ''")
+	}
+
+	sw.envs = []string{"foo=bar"}
+	if sw.getEnv("foo") != "bar" {
+		t.Fatal("5 - 'foo' should map to 'bar'")
+	}
+
+	sw.envs = []string{"foo=bar", "car=hat"}
+	if sw.getEnv("foo") != "bar" {
+		t.Fatal("6 - 'foo' should map to 'bar'")
+	}
+	if sw.getEnv("car") != "hat" {
+		t.Fatal("7 - 'car' should map to 'hat'")
+	}
+
+	// Make sure we grab the first 'car' in the list
+	sw.envs = []string{"foo=bar", "car=hat", "car=bike"}
+	if sw.getEnv("car") != "hat" {
+		t.Fatal("8 - 'car' should map to 'hat'")
+	}
+}
+
+func TestProcessWords(t *testing.T) {
+	test := "some content 'x foo x' \"a string arg\""
+	words, err := ProcessWords(test, []string{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if words[0] != "some" {
+		t.Fatalf("%q != %q", words[0], "some")
+	}
+	if words[1] != "content" {
+		t.Fatalf("%q != %q", words[1], "content")
+	}
+	if words[2] != "'x foo x'" {
+		t.Fatalf("%q != %q", words[2], "'x foo x'")
+	}
+	if words[3] != "\"a string arg\"" {
+		t.Fatalf("%q != %q", words[3], "\"a string arg\"")
+	}
+}