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

panic when marshalling a map with the key in a custom string type #413

Closed
falau opened this issue Jun 4, 2024 · 0 comments · Fixed by #414
Closed

panic when marshalling a map with the key in a custom string type #413

falau opened this issue Jun 4, 2024 · 0 comments · Fixed by #414

Comments

@falau
Copy link

falau commented Jun 4, 2024

Env

  • Go version: 1.22.3
  • BurntSushi/toml@v1.4.0

Description

When marshalling a map with keys in a custom string type like:

type CustomString string
type MyMap map[CustomString]int

func main() {
	var myKey CustomString = "MyKey"
	m := MyMap{myKey: 10}
	toml.NewEncoder(os.Stdout).Encode(m)
}

it turns out that the encoder simply uses a primitive string to set the map key instead of the CustomString type here, and causes panic.
As both encoding/json and goyaml can handle this case without any problem, I expect this toml package should also work in the same way?
If not, is there any way to do this properly?

Thanks

Stacktrace

panic: reflect.Value.MapIndex: value of type string is not assignable to type main.CustomString [recovered]
	panic: reflect.Value.MapIndex: value of type string is not assignable to type main.CustomString

goroutine 1 [running]:
github.com/BurntSushi/toml.(*Encoder).safeEncode.func1()
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:160 +0x73
panic({0xfb7b680?, 0xc0000140d0?})
	/usr/local/Cellar/go/1.22.3/libexec/src/runtime/panic.go:770 +0x132
reflect.Value.assignTo({0xfb7b680?, 0xc0000140c0?, 0xfc1ba80?}, {0xfb4ecf7, 0x16}, 0xfb7b640, 0x0)
	/usr/local/Cellar/go/1.22.3/libexec/src/reflect/value.go:3356 +0x299
reflect.Value.MapIndex({0xfb80360?, 0xc0000760c0?, 0x0?}, {0xfb7b680, 0xc0000140c0, 0x98})
	/usr/local/Cellar/go/1.22.3/libexec/src/reflect/value.go:1821 +0xe8
github.com/BurntSushi/toml.(*Encoder).eMap.func1({0xc0000140b0, 0x1, 0x2?}, 0x0)
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:418 +0x154
github.com/BurntSushi/toml.(*Encoder).eMap(0xc00010df20, {0xfc7a980, 0x0, 0x0}, {0xfb80360?, 0xc0000760c0?, 0xfb712f0?}, 0x0)
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:437 +0x42c
github.com/BurntSushi/toml.(*Encoder).eMapOrStruct(0xfb06f32?, {0xfc7a980?, 0xfacba9b?, 0xc00010dce0?}, {0xfb80360?, 0xc0000760c0?, 0xfb80360?}, 0x20?)
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:388 +0x4c
github.com/BurntSushi/toml.(*Encoder).eTable(0xc00010df20, {0xfc7a980, 0x0, 0x0}, {0xfb80360?, 0xc0000760c0?, 0x0?})
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:382 +0x19c
github.com/BurntSushi/toml.(*Encoder).encode(0xc00010df20, {0xfc7a980, 0x0, 0x0}, {0xfb80360?, 0xc0000760c0?, 0xc000100680?})
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:203 +0x1b5
github.com/BurntSushi/toml.(*Encoder).safeEncode(0xfb80360?, {0xfc7a980?, 0x0?, 0xfc1c100?}, {0xfb80360?, 0xc0000760c0?, 0x40?})
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:163 +0x6c
github.com/BurntSushi/toml.(*Encoder).Encode(0xc00010df20, {0xfb80360?, 0xc0000760c0?})
	/go/pkg/mod/github.com/!burnt!sushi/toml@v1.4.0/encode.go:146 +0x9c

main.main()
	/myrepo/mygo/cmd/mytest/main.go:16 +0x10f
exit status 2
arp242 added a commit that referenced this issue Jun 4, 2024
Because the map keys were converted to []string,
rv.MapIndex(reflect.ValueOf(mapKey)) would panic as it's the "wrong"
type (string, instead of the custom string type).

Just keep track of the reflect.Value instead; don't really need to
convert it to strings.

Fixes #413
arp242 added a commit that referenced this issue Jun 4, 2024
Because the map keys were converted to []string,
rv.MapIndex(reflect.ValueOf(mapKey)) would panic as it's the "wrong"
type (string, instead of the custom string type).

Just keep track of the reflect.Value instead; don't really need to
convert it to strings.

Fixes #413
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.

1 participant