diff --git a/plugins/config/src/configenv/configenv.go b/plugins/config/src/configenv/configenv.go index d449c7bbe..65ae6884a 100644 --- a/plugins/config/src/configenv/configenv.go +++ b/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 diff --git a/plugins/config/src/configenv/configenv_test.go b/plugins/config/src/configenv/configenv_test.go index d0b3cbf02..a948f865f 100644 --- a/plugins/config/src/configenv/configenv_test.go +++ b/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 { diff --git a/plugins/config/src/configenv/parsing.go b/plugins/config/src/configenv/parsing.go index bf539156b..3b30f6d9c 100644 --- a/plugins/config/src/configenv/parsing.go +++ b/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