Populating strings with values that could be parsed as booleans mutates the result

[lrewega]

package main

import (
        "fmt"

        "go.uber.org/config"
)

func main() {
        p, err := config.NewYAMLProviderFromBytes([]byte(`foo: yes`))
        if err != nil {
                panic(err)
        }

        var foo string

        if err := p.Get("foo").Populate(&foo); err != nil {
                panic(err)
        }

        fmt.Println(foo)
}

This prints true, not yes as I'd expect.

[glibsm]

Good find. foo: "yes" does the right thing, but it should work with foo: yes correctly as well.

The underlying yaml library does the right thing, which likely means a bug in config code somewhere.

package main

import (
	"fmt"
	"log"

	"gopkg.in/yaml.v2"
)

func main() {
	var stringFoo string
	err := yaml.Unmarshal([]byte(`yes`), &stringFoo)
	if err != nil {
		log.Panic(err)
	}
	fmt.Println("string foo", stringFoo)

	var boolFoo bool
	err = yaml.Unmarshal([]byte(`yes`), &boolFoo)
	if err != nil {
		log.Panic(err)
	}
	fmt.Println("bool foo", boolFoo)
}

[glibsm]

It looks like at the core is the way yaml library handles interaface{} unmarshaling. Here is the smallest repro scenario for this bug:

package main

import (
	"fmt"
	"log"

	"gopkg.in/yaml.v2"
)

func main() {
	var i interface{}
	err := yaml.Unmarshal([]byte(`yes`), &i)
	if err != nil {
		log.Panic(err)
	}
	fmt.Println("interface{} foo", i)
}
// Output: interface{} foo true

The line in question: https://github.com/uber-go/config/blob/master/yaml.go#L51

[xuyang2]

YAML 1.1 says:

Canonical:

y|n

Regexp:

y|Y|yes|Yes|YES|n|N|no|No|NO |true|True|TRUE|false|False|FALSE |on|On|ON|off|Off|OFF

Related lines in yaml library:

• https://github.com/go-yaml/yaml/blob/5420a8b6744d3b0345ab293f6fcba19c978f1183/resolve.go#L37

{true, yaml_BOOL_TAG, []string{"y", "Y", "yes", "Yes", "YES"}},

• https://github.com/go-yaml/yaml/blob/5420a8b6744d3b0345ab293f6fcba19c978f1183/yamlh.go#L310

yaml_BOOL_TAG      = "tag:yaml.org,2002:bool"      // The tag !!bool with the values: true and false.

So I think this can be work around by adding an explicit !!str tag:

package main

import (
	"fmt"
	"log"

	"gopkg.in/yaml.v2"
)

func main() {
	var i interface{}
	err := yaml.Unmarshal([]byte(`!!str yes`), &i)
	if err != nil {
		log.Panic(err)
	}
	fmt.Println("interface{} foo", i)
}
// Output: interface{} foo yes

[akshayjshah]

As @xuyang2 points out, the YAML specification outlines a number of different ways to encode Boolean values. The way these strings are treated depends on the structure they're unmarshalled into - unmarshalling into a string keeps the YAML as-is, while unmarshalling into interface{} treats these strings as Booleans. Quoting these special strings or using !!str forces standards-compliant YAML parsers (including gopkg.in/yaml.v2) to always treat them as strings, side-stepping this issue entirely.

These special representations of bools cause particular trouble for this package because we need to merge multiple YAML files into a single file. Post-v1.1, we do this decoding into interface{}, merging, and re-encoding. (We handle it a little differently in earlier versions, but the bug is the same.) This has the unfortunate side effect of converting these special strings into "true" or "false" even when unmarshalling into a string field. This is unfortunate, but it's difficult to fix without introducing more significant breakages in multi-source configurations.

In principle, we could fix this by writing a YAML parser from scratch and merging ASTs without unmarshalling. I doubt that we'll ever spend the time required to actually do this.

[akshayjshah]

Documented this more clearly in #94. I'm going to leave this open in case someone comes up with a nice fix (or feels like writing a fully-compliant YAML parser that lets us merge without unmarshalling).