Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error Unmarshal map when key contains dot #324

Open
StephenPCG opened this issue Mar 30, 2017 · 19 comments · Fixed by #673
Open

Error Unmarshal map when key contains dot #324

StephenPCG opened this issue Mar 30, 2017 · 19 comments · Fixed by #673

Comments

@StephenPCG
Copy link

Test code:

package main

import (
	"bytes"
	"fmt"
	"github.com/spf13/viper"
)

type Config struct {
	Foo struct {
		Bar map[string]int `mapstructure:"bar"`
	} `mapstructure:"foo"`
}

var content = []byte(`
foo:
  bar:
    "x": 1
    "y": 2
    "z.z": 3
`)

func main() {
	var c1, c2 Config

	viper.SetConfigType("yaml")
	viper.ReadConfig(bytes.NewReader(content))

	viper.Unmarshal(&c1)
	fmt.Println("c1:", c1)

	viper.UnmarshalKey("foo", &c2.Foo)
	fmt.Println("c2:", c2)
}

Output:

c1: {{map[x:1 y:2]}}
c2: {{map[x:1 y:2 z.z:3]}}

As you can see, when the map key contains dot, UnmarshalKey works correctly but Unmarshal doesn't.

@dlouwers
Copy link

This still seems to be an issue for me. This test fails depending on v1.3.1:

package main

import (
	"strings"
	"testing"

	"github.com/spf13/viper"

	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestConfigMarshalling(t *testing.T) {
	data := `
root:
  ele.one: value
  eletwo: value`
	reader := strings.NewReader(data)
	viper.SetConfigType("yaml")
	err := viper.ReadConfig(reader)
	require.NoError(t, err)

	// Test if the fields are even correct
	expected := map[string]interface{}{"ele.one": "value", "eletwo": "value"}
	assert.Equal(t, expected, viper.Get("root"))
	var s struct {
		Root map[string]string
	}
	err = viper.UnmarshalKey("root", &s)
	require.NoError(t, err)
}

Changing the dot to, say, an underscore fixes the issue.

@alexeldeib
Copy link

Also hit this. Seems like if there's a key like "word.other" containing a dot as a map[string]interface{} key, the output turns it into a struct like:

{
    "word": {
        "other": "value"      
    }
}

and not

{
    "word.other": "value"
}

Is that expected? Anyone have pointers to override this to get the former behavior?

@inkychris
Copy link
Contributor

inkychris commented Mar 13, 2019

Looks like AllKeys() returns a list of all the keys it can find as delimited strings:

// main.go
func main() {
	configString := `
test:
  a: 1 
  b: 2
  c.d: 3
`
	viper.SetConfigType("yaml")
	err := viper.ReadConfig(bytes.NewBufferString(configString))
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
	fmt.Println(viper.AllKeys())
}
[test.c.d test.a test.b]

As noted, the problem comes from keys with dots in as the unmarshalling uses the keys returned by AllKeys() and the default key delimiter is a dot, causing it to incorrectly interpret a dot in a key name for a sub key.

I added the following functions to viper.go in order to change the delimiter. Alternatively you could just change the default delimiter in New():

// viper.go
func SetKeyDelim(delim string) { v.keyDelim = delim }
func (v *Viper) SetKeyDelim(delim string) { v.keyDelim = delim }
// main.go
func main() {
	configString := `
test:
  a: 1 
  b: 2
  c.d: 3
`
	viper.SetKeyDelim("\\")
	viper.SetConfigType("yaml")
	err := viper.ReadConfig(bytes.NewBufferString(configString))
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}
	fmt.Println(viper.AllKeys())
}
[test\c.d test\a test\b]

The simplest solution would of course be to change the default but perhaps for the sake of backward compatibility, it would be best to just add the option to change the delimiter.
Given that the dot is used to allow you to traverse through keys, it would be pragmatic to just add the option to change the delimiter on a case by case basis.

@mschneider82
Copy link

hopefully it will get merged soon!

@inkychris
Copy link
Contributor

I will also say that despite it's simplicity, I imagine adding to the API is not going to be as favourable as fixing the implementation of Unmarshal in a more thorough way. The fact that keys with dots in can be handled successfully via the viper.GetStringMap_ methods suggests that there is probably a better way to fix this such that it's only a patch rather than minor change, semantically speaking (even though it would probably be a bigger change). I will continue to look into this but I'm new to both Go and the project so can't promise anything!

@luisdavim
Copy link

I wonder, if we use dots in a key and AutomaticEnv, how will the env variable names look like?

@inkychris
Copy link
Contributor

I'll have to look into exactly what AutomaticEnv is and what it does, I'm still fairly new to the project but I'm guessing the issue is going to be with environment variables containing dots which is syntactically incorrect ?

@luisdavim
Copy link

Yes. Also #678

xixuejia added a commit to xixuejia/viper that referenced this issue Apr 9, 2019
@inkychris
Copy link
Contributor

@sagikazarmark is it worth opening this again?

@sagikazarmark
Copy link
Collaborator

I guess it does, since #673 was reverted.

@sagikazarmark sagikazarmark reopened this Sep 28, 2019
@gustavooferreira
Copy link

Hey guys, I'm hitting this problem too. Need to get a map with the key being IPs, but it fails.

I've been using this library for quite a long time now for pretty much all my personal and work projects.

Any chance we can get some brainstorming on this?

@mschneider82
Copy link

mschneider82 commented Nov 20, 2019

You could stick your viper module to commit 99520c8 this commit was reverted but you don't need to run on latest :)
go get github.com/spf13/viper@99520c8

@gustavooferreira
Copy link

@mschneider82 thank you for the tip!

@inkychris
Copy link
Contributor

The alternative is to change the key delimiter which will be added in #794, this is on a branch of this repo and I assume will be merged at some point so could also lock the go module to that reference.

As for a more robust solution I've been thinking about a way forward. As discussed in #766, the problem is being unable to determine from a single string, whether a given character should be part of the key name or is treated as the delimiter, eg: sites.example.com.port can be interpreted many different ways. The reason it can work for unmarshalling, which is what I tried to fix, is that you're telling viper the config schema in the form of a struct.

I believe the only way to also solve the problem for environment variable and pflag bindings is to provide viper with some sort of schema (like we do for unmarshalling), so that it knows explicitly what to do with a parameter address that might contain delimiters within key names, rather than assuming they're all separate keys. I'm not yet sure if this is something that could be achieved within v1, perhaps providing a schema would be like the feature toggle @sagikazarmark mentions in #794 (comment).

@sagikazarmark
Copy link
Collaborator

Right, I fully intend to merge #794 soon, but I'm playing with replacing the setter with a constructor/factory function argument. I don't really like setters, and in this case it could actually mess up the internal state of Viper, so that's what blocks the PR from getting merged.

@WoLfulus
Copy link

WoLfulus commented Jun 6, 2020

Any workarounds in the meantime?

@inkychris
Copy link
Contributor

@WoLfulus With #794 now merged into master, the current workaround is to set your delimiter to something other than a dot.

@kushmittal
Copy link

I am having issues while creating TOML file.
I set keyDelimiter as "!" but still when key contains "dot".

It add additional double quotes as below:
["x,y"]
instead of [x.y]

@gusandrioli
Copy link

I have a very similar problem but with keys with _ instead. Any key with _ does not get parsed on UnmarshalKey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.