uber-go / config

Configuration for Go applications
https://godoc.org/go.uber.org/config
MIT License
442 stars 41 forks source link

Env var expansion logic inconsistent #101

Open bayesianmind opened 6 years ago

bayesianmind commented 6 years ago

We've seen several users be quite confused about the defaults section of environment variable expansion defined here.

The behavior around quote characters is inconsistent and this has caused user confusion. ${NOTSET:} - fails saying no default has been provided. ${NOTSET:""} - uses empty string and strips the quote characters. ${NOTSET:"value"} - returns "value" with the quotes, usually breaking the customer's usage. It's very natural for them to take a "" default and fill in a string inside the quotes. ${NOTSET:value} - returns value as the user probably intended.

Quotes should either always be stripped or never be stripped.

Option 1: The most backwards compatible change is simply to allow the form ${NOTSET:} but this is more confusing because "" will still really mean empty string while "a" will mean quoted "a".

Option 2: Strip outer quotes. Potentially a breaking change in the unlikely case people actually want literal quote characters in their defaults. Better long term outcome because behavior is totally consistent and all forms of defaults are supported. If you want quotes, you would escape them like ${NOTSET:"\\"quoted\\""}

glibsm commented 6 years ago

Linked code looked like something I wrote a while back, so I went digging. Found the original PR https://github.com/uber-go/fx/pull/384

Can't quite remember the reason why ${NOTSET:} is treated explicitly as invalid.