From 78cbb5ce2e3889831777ae12ee28d67d14668413 Mon Sep 17 00:00:00 2001 From: Long Nguyen Date: Tue, 4 Jan 2022 15:03:25 -0500 Subject: [PATCH] Ensure quote stripping behavior for --var matches legacy behaviour. - We should only remove the first layer of matching quotes. - We should continue to throw errors when a string cannot be yaml unmarshalled. (There hasn't been a request to change this behaviour and the impact is unknown) Signed-off-by: Rajan Agaskar Signed-off-by: Long Nguyen --- director/template/var_kv.go | 13 +++++++----- director/template/var_kv_test.go | 35 +++++++++++++++++++------------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/director/template/var_kv.go b/director/template/var_kv.go index 6a3225b1b..b5492092d 100644 --- a/director/template/var_kv.go +++ b/director/template/var_kv.go @@ -15,7 +15,6 @@ type VarKV struct { func trimWrappingQuotes(s string) string { if (s[0] == '"' && s[len(s)-1] == '"') || (s[0] == '\'' && s[len(s)-1] == '\'') { s = s[1 : len(s)-1] - s = trimWrappingQuotes(s) } return s } @@ -37,11 +36,15 @@ func (a *VarKV) UnmarshalFlag(data string) error { var vars interface{} err := yaml.Unmarshal([]byte(pieces[valueIndex]), &vars) - //If for whatever reason YAML unmarshal fails, treat the second part of pieces as a string + + if err != nil { + return bosherr.WrapErrorf(err, "Deserializing variables '%s'", data) + } + //yaml.Unmarshal returns a string if the input is not valid yaml. - if _, ok := vars.(string); ok || err != nil { - //in that case, we return the initial flag value as the Unmarshal - //process strips newlines. Stripping the quotes is required to keep + //in that case, we pass through the string itself as the Unmarshal process strips newlines. + if _, ok := vars.(string); ok { + //Stripping the quotes is required to keep //backwards compability (YAML unmarshal also removed wrapping quotes from the value). *a = VarKV{Name: pieces[nameIndex], Value: trimWrappingQuotes(pieces[valueIndex])} } else { diff --git a/director/template/var_kv_test.go b/director/template/var_kv_test.go index 1f4ccc604..234382b63 100644 --- a/director/template/var_kv_test.go +++ b/director/template/var_kv_test.go @@ -23,32 +23,39 @@ var _ = Describe("VarKV", func() { Expect(arg).To(Equal(VarKV{Name: "name", Value: "val"})) }) - It("reverts to the old (incorrect) bevaviour and removes the wrapping double quotes if the value is a string", func() { + It("reverts to the old (incorrect) behaviour and removes the wrapping double quotes if the value is a string", func() { err := (&arg).UnmarshalFlag(`name="val"`) Expect(err).ToNot(HaveOccurred()) Expect(arg).To(Equal(VarKV{Name: "name", Value: "val"})) }) - It("reverts to the old (incorrect) bevaviour and removes the wrapping single quotes if the value is a string", func() { + It("reverts to the old (incorrect) behaviour and removes the wrapping single quotes if the value is a string", func() { err := (&arg).UnmarshalFlag(`name='val'`) Expect(err).ToNot(HaveOccurred()) Expect(arg).To(Equal(VarKV{Name: "name", Value: "val"})) }) - It("only removes wrapping quotes", func() { - err := (&arg).UnmarshalFlag(`name="'val""val'"`) + It("fails if the string starts with unmatched quoting", func() { + err := (&arg).UnmarshalFlag(`name="my-password'`) + Expect(err).To(HaveOccurred()) + }) + It("only ever removes a single layer of wrapping quotes", func() { + err := (&arg).UnmarshalFlag(`name='""'`) Expect(err).ToNot(HaveOccurred()) - Expect(arg).To(Equal(VarKV{Name: "name", Value: `val""val`})) - - err = (&arg).UnmarshalFlag(`name="val''val"`) + Expect(arg).To(Equal(VarKV{Name: "name", Value: `""`})) + }) + It("only removes wrapping quotes on an empty quoted value", func() { + err := (&arg).UnmarshalFlag(`name="''"`) Expect(err).ToNot(HaveOccurred()) - Expect(arg).To(Equal(VarKV{Name: "name", Value: `val''val`})) - - err = (&arg).UnmarshalFlag(`name='"some-data'"`) + Expect(arg).To(Equal(VarKV{Name: "name", Value: `''`})) + }) + It("only removes wrapping quotes on a quoted value", func() { + err := (&arg).UnmarshalFlag(`name='"mocked-value"'`) Expect(err).ToNot(HaveOccurred()) - Expect(arg).To(Equal(VarKV{Name: "name", Value: `'"some-data'"`})) - - err = (&arg).UnmarshalFlag(`name="'some-data"'`) + Expect(arg).To(Equal(VarKV{Name: "name", Value: `"mocked-value"`})) + }) + It("removes the quotes on a quoted blank string", func() { + err := (&arg).UnmarshalFlag(`name=''`) Expect(err).ToNot(HaveOccurred()) - Expect(arg).To(Equal(VarKV{Name: "name", Value: `"'some-data"'`})) + Expect(arg).To(Equal(VarKV{Name: "name", Value: ``})) }) It("sets name and value when value contains a `=`", func() { err := (&arg).UnmarshalFlag("public_key=ssh-rsa G4/+VHa1aw==")