Skip to content

Commit

Permalink
templateargs: Deal with YAML's handling of "y" as boolean (#254)
Browse files Browse the repository at this point in the history
When using yab templates and a key is specified as `"y"` (with quoting),
the template parsing ends up converting this to `y`, which then gets
unmarshalled as a boolean. The underlying cause is:
go-yaml/yaml#214

This is caused by template rendering's use of `yaml.Unmarshal` on all
string values which can lose the type.

This change restricts the `Unmarshal` to values that were modified
as part of the rendering step. It also adds special logic to protect
against YAML's boolean handling, by only using the boolean value
if strconv.ParseBool would parse the value. This means if a value
was rendered as `y`, we would still not treat it as a boolean.

Fixes #253
  • Loading branch information
prashantv committed Apr 16, 2019
1 parent 0f1beae commit a02e823
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 2 deletions.
17 changes: 17 additions & 0 deletions templateargs/process.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package templateargs

import (
"strconv"

"github.com/yarpc/yab/templateargs/interpolate"

"gopkg.in/yaml.v2"
Expand Down Expand Up @@ -29,10 +31,25 @@ func processString(v string, args map[string]string) (interface{}, error) {
if rendered == "" {
return "", nil
}
if rendered == v {
// Avoid unmarshalling if the value did not change.
return v, nil
}

// Otherwise, unmarshal the value and return that.
var unmarshalled interface{}
err = yaml.Unmarshal([]byte(rendered), &unmarshalled)

if _, isBool := unmarshalled.(bool); isBool {
// The Go YAML parser has some unfortunate handling of strings that look
// like booleans: https://github.com/go-yaml/yaml/issues/214
// Let's use a more strict definition for booleans.
if _, err := strconv.ParseBool(rendered); err != nil {
// Go doesn't think this is a boolean, so use the value as a string
return rendered, nil
}
}

return unmarshalled, err
}

Expand Down
23 changes: 21 additions & 2 deletions templateargs/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@ type (

func TestProcessString(t *testing.T) {
args := map[string]string{
"user": "prashant",
"count": "10",
"user": "prashant",
"count": "10",
"y": "y",
"t": "true",
"t_quoted": `"true"`,
}
tests := []struct {
v string
Expand All @@ -28,6 +31,22 @@ func TestProcessString(t *testing.T) {
v: "s",
want: "s",
},
{
v: "y",
want: "y",
},
{
v: "${y}",
want: "y",
},
{
v: "${t}",
want: true,
},
{
v: "${t_quoted}",
want: "true",
},
{
v: "${unknown}",
wantErr: `unknown variable "unknown"`,
Expand Down

0 comments on commit a02e823

Please sign in to comment.