[config] change file format to match bash exports

got rid of nonstandard escaping/quoting so that
the disk file is a strict subset of valid bash exports

plugins/config/src/configenv/configenv.go

@@ -37,13 +37,13 @@ func (e *Env) stringWithEntryPrefix(prefix string) string {
 	sort.Strings(keys)
 	entries := make([]string, len(keys))
 	for i, k := range keys {
-		entries[i] = fmt.Sprintf("%s%s='%s'", prefix, k, escape(e.env[k]))
+		entries[i] = fmt.Sprintf("%s%s='%s'", prefix, k, singleQuoteEscape(e.env[k]))
 	}
 	return strings.Join(entries, "\n")
 }
 
-func escape(value string) string { //TODO: improve this
-	return strings.Replace(value, "'", "\\'", -1)
+func singleQuoteEscape(value string) string { // so that 'esc'apped' -> 'esc'\''aped'
+	return strings.Replace(value, "'", "'\\''", -1)
 }
 
 //NewFromTarget creates an env from the given target. Tareget is either "--global" or an app name

plugins/config/src/configenv/configenv_test.go

@@ -6,27 +6,42 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestBasic(t *testing.T) {
+func TestExportfileRoundtrip(t *testing.T) {
 	RegisterTestingT(t)
-	env, err := NewFromString("HI=ho")
+	env, err := NewFromString("HI='ho'\nFoo='Bar'\n\nBaz='BOFF'")
 	Expect(err).NotTo(HaveOccurred())
-	Expect(env.Map()).To(Equal(pairs("HI", "ho")))
-	Expect(env.String()).To(Equal("HI='ho'"))
+	Expect(env.Map()).To(Equal(pairs("Baz", "BOFF", "Foo", "Bar", "HI", "ho")))
+	Expect(env.String()).To(Equal("Baz='BOFF'\nFoo='Bar'\nHI='ho'"))
 
-	env, err = NewFromString("HI=h\\o\\'")
+	env, err = NewFromString("\n export HI='h\\\no\\'\n")
 	Expect(err).NotTo(HaveOccurred())
-	Expect(env.Map()).To(Equal(pairs("HI", "h\\o'")))
-	Expect(env.String()).To(Equal("HI='h\\o\\''"))
+	Expect(env.Map()).To(Equal(pairs("HI", "h\\\no\\")))
+	Expect(env.String()).To(Equal("HI='h\\\no\\'"))
 
-	env, err = NewFromString("HI=ho\nFOO=bar\n\n")
+	env, err = NewFromString("HI='ho'\nFOO=''\\''\nBAR='\\'''\\'''")
 	Expect(err).NotTo(HaveOccurred())
-	Expect(env.Map()).To(Equal(pairs("HI", "ho", "FOO", "bar")))
-	Expect(env.String()).To(Equal("FOO='bar'\nHI='ho'"))
+	Expect(env.Map()).To(Equal(pairs("HI", "ho", "FOO", "'\nBAR=''")))
+	Expect(env.String()).To(Equal("FOO=''\\''\nBAR='\\'''\\'''\nHI='ho'"))
 
-	env, err = NewFromString("HI='ho\n'\n\nFOO='bar\\'\nbaz'")
+	env, err = NewFromString("HI='ho\n'\n\nFOO='=bar\"\n'\\''\nbaz'")
 	Expect(err).NotTo(HaveOccurred())
-	Expect(env.Map()).To(Equal(pairs("HI", "ho\n", "FOO", "bar'\nbaz")))
-	Expect(env.String()).To(Equal("FOO='bar\\'\nbaz'\nHI='ho\n'"))
+	Expect(env.Map()).To(Equal(pairs("HI", "ho\n", "FOO", "=bar\"\n'\nbaz")))
+	Expect(env.String()).To(Equal("FOO='=bar\"\n'\\''\nbaz'\nHI='ho\n'"))
+}
+
+func TestExportfileErrors(t *testing.T) {
+	RegisterTestingT(t)
+	_, err := NewFromString("FOO=bar") //values must be quoted
+	Expect(err).To(HaveOccurred())
+
+	_, err = NewFromString("FOO='bar\\''") //single quotes are not escaped this way
+	Expect(err).To(HaveOccurred())
+
+	_, err = NewFromString("F\nOO='bar'") //keys cannot have embedded newlines
+	Expect(err).To(HaveOccurred())
+
+	_, err = NewFromString("FOO='bar' ") //no trailing content
+	Expect(err).To(HaveOccurred())
 }
 
 func pairs(vars ...string) map[string]string {

plugins/config/src/configenv/parsing.go

@@ -24,15 +24,13 @@ func parseEnv(name string, filename string) (*Env, error) {
 }
 
 func parseEnvFromReader(name string, filename string, reader io.Reader) (*Env, error) {
-	//The file format looks like KEY='VALUE'. Eait pair is terminated with a newline.
-	//All characters are valid for VALUE without escaping
-	//but one: the single quote and backslash. To represent a single quote emit \'. To represent
-	//a literal backslash emit \\. In any other case, the backslash is interpreted literally.
-	//KEY='value
-	//one'
-	//KEY='val\\ue\\'' etc..
+	//The file format looks like: `KEY='VALUE'`. Eait pair is terminated with a newline.
+	//All characters are valid for VALUE without escaping but one: the single quote.
+	//A single quote literal is represented by ending the quoting
+	//emitting a \' and resuming the quoting. So for VALUE to be "don't care":
+	//`KEY='don'\''t care'`
+	//The advantage of this is that it's easily parsed and sourced by bash correctly.
 
-	//TODO: We might want to rework this to use shell-style quoting for single quotes
 	buffered := bufio.NewReader(reader)
 	const (
 		StateKey   = iota
@@ -50,9 +48,11 @@ func parseEnvFromReader(name string, filename string, reader io.Reader) (*Env, e
 			switch char {
 			case ' ':
 				if buffer.String() == "export" {
-					buffer.Truncate(0) //so we can read exportfiles too
+					buffer.Truncate(0) //so we can read exportfiles as well as envfiles
+				} else if buffer.Len() == 0 {
+					continue //leading spaced are allowed but not encouraged
 				} else {
-					return nil, errors.New("Env keys cannot have spaces")
+					return nil, errors.New("keys cannot have spaces")
 				}
 			case '=':
 				key = buffer.String()
@@ -60,7 +60,7 @@ func parseEnvFromReader(name string, filename string, reader io.Reader) (*Env, e
 				state = StateValue
 			case '\n':
 				if buffer.Len() > 0 {
-					return nil, errors.New("Invalid newline after: " + buffer.String())
+					return nil, errors.New("keys cannot contain newlines")
 				}
 			default:
 				buffer.WriteRune(char)
@@ -75,7 +75,7 @@ func parseEnvFromReader(name string, filename string, reader io.Reader) (*Env, e
 					quoted = !quoted
 				}
 			case '\n':
-				if escaped || quoted {
+				if quoted {
 					buffer.WriteRune(char)
 				} else {
 					state = StateKey
@@ -86,33 +86,35 @@ func parseEnvFromReader(name string, filename string, reader io.Reader) (*Env, e
 					quoted = false
 				}
 			case '\\':
-				if escaped {
+				if quoted {
 					buffer.WriteRune(char)
 					escaped = false
 				} else {
 					escaped = true
 				}
 			default:
+				if !quoted {
+					return nil, errors.New("unquoted or unbalanced value for " + key)
+				}
 				if escaped {
-					buffer.WriteRune('\\')
-					escaped = false
+					return nil, errors.New("invalid escape for " + key)
 				}
 				buffer.WriteRune(char)
 			}
 		}
 	}
 	if escaped {
-		return nil, errors.New("Unterminated escape")
+		return nil, errors.New("unterminated escape at end")
 	}
 	if quoted {
-		return nil, errors.New("Unterminated quote")
+		return nil, errors.New("unterminated quote at end")
 	}
 
 	if state == StateValue {
 		env.Set(key, buffer.String())
 	} else {
 		if buffer.Len() > 0 {
-			return nil, errors.New("Invalid trailing content: " + buffer.String())
+			return nil, errors.New("invalid trailing content: '" + buffer.String() + "'")
 		}
 	}
 	return env, nil