From a02e823eeca4acd5ae7544b65ac05a451a700c2b Mon Sep 17 00:00:00 2001
From: Prashant Varanasi <github@prashantv.com>
Date: Tue, 16 Apr 2019 16:00:16 -0700
Subject: [PATCH] templateargs: Deal with YAML's handling of "y" as boolean
 (#254)

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:
https://github.com/go-yaml/yaml/issues/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
---
 templateargs/process.go      | 17 +++++++++++++++++
 templateargs/process_test.go | 23 +++++++++++++++++++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/templateargs/process.go b/templateargs/process.go
index e387884d..6ebc5f4d 100644
--- a/templateargs/process.go
+++ b/templateargs/process.go
@@ -1,6 +1,8 @@
 package templateargs
 
 import (
+	"strconv"
+
 	"github.com/yarpc/yab/templateargs/interpolate"
 
 	"gopkg.in/yaml.v2"
@@ -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
 }
 
diff --git a/templateargs/process_test.go b/templateargs/process_test.go
index 913d3f5d..dcf801b5 100644
--- a/templateargs/process_test.go
+++ b/templateargs/process_test.go
@@ -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
@@ -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"`,