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

Seeing "invalid TOML syntax" for valid TOML #10534

Closed
lorenmh opened this issue Jan 27, 2022 · 2 comments · Fixed by #14875
Closed

Seeing "invalid TOML syntax" for valid TOML #10534

lorenmh opened this issue Jan 27, 2022 · 2 comments · Fixed by #14875
Assignees
Labels
bug unexpected problem or unintended behavior help wanted Request for community participation, code, contribution size/l 1 week or more effort

Comments

@lorenmh
Copy link

lorenmh commented Jan 27, 2022

Relevent telegraf.conf

my_array = [
  # hello I am a comment
  "hello",
]

Logs from Telegraf

invalid TOML syntax

System info

master branch telegraf

Docker

No response

Steps to reproduce

check out my fork here: https://github.com/lorenmh/telegraf
run go test ./config/...

diff here: lorenmh@7e44c12

Expected behavior

This is valid TOML and it should be parsed correctly

Actual behavior

Parse fails, Telegraf won't run

Additional info

No response

@lorenmh lorenmh added the bug unexpected problem or unintended behavior label Jan 27, 2022
@powersj
Copy link
Contributor

powersj commented Jan 27, 2022

I consider BurntSushi/toml the canonical source, this does appear to work and it is valid TOML syntax:

package main

import (
	"fmt"
	"log"

	"github.com/BurntSushi/toml"
)

var tomlData = `
array = [
  # hello I am a comment
  "hello",
  "world"
]
`

type tomlConfig struct {
	Array []string `toml:"array"`
}

func main() {
	var conf tomlConfig
	if _, err := toml.Decode(tomlData, &conf); err != nil {
		log.Fatal(err)
	}

	fmt.Println(conf)
}
$  go build && ./test 
{[hello world]}

However, we use the https://github.com/influxdata/toml library to parse TOML files. This is really a fork of https://github.com/naoina/toml. I am able to reproduce the invalid TOML syntax with the last release of both libraries:

❯ go mod tidy
go: finding module for package github.com/influxdata/toml
go: downloading github.com/influxdata/toml v0.0.0-20180607005434-2a2e3012f7cf
go: found github.com/influxdata/toml in github.com/influxdata/toml v0.0.0-20180607005434-2a2e3012f7cf
go: finding module for package github.com/naoina/go-stringutil
go: found github.com/naoina/go-stringutil in github.com/naoina/go-stringutil v0.1.0
~/test via 🐹 v1.17.6 
❯ go build && ./test 
2022/01/27 12:49:07 toml: line 3: parse error
~/test via 🐹 v1.17.6 
❯ go mod tidy
go: finding module for package github.com/naoina/toml
go: downloading github.com/naoina/toml v0.1.1
go: found github.com/naoina/toml in github.com/naoina/toml v0.1.1
go: finding module for package github.com/kylelemons/godebug/pretty
go: finding module for package github.com/kylelemons/godebug/diff
go: found github.com/kylelemons/godebug/diff in github.com/kylelemons/godebug v1.1.0
go: found github.com/kylelemons/godebug/pretty in github.com/kylelemons/godebug v1.1.0
~/test via 🐹 v1.17.6 
❯ go build && ./test 
2022/01/27 12:52:12 line 3: invalid TOML syntax

However, once I upgrade the library to master it is able to parse the TOML:

❯ go get github.com/naoina/toml@master
go: downloading github.com/naoina/toml v0.1.2-0.20210730182554-e80af6068b28
go get: upgraded github.com/naoina/toml v0.1.1 => v0.1.2-0.20210730182554-e80af6068b28
go build && ./test 

array = [
  # hello I am a comment
  "hello",
  "world"
]

It looks like we are missing naoina/toml@605f287 this commit for comments in arrays.

Next steps: look to update our fork with the missing commit or commits from the https://github.com/naoina/toml library

@sspaink
Copy link
Contributor

sspaink commented Apr 25, 2022

Looks like this is a duplicate of: #3642, I went ahead and closed #3642 in favor of this one because I think this issue has more information to help resolve the problem.

It would be nice if we could switch to BurntSushi/toml, from the comments in #3642 and #6474 it seems this was the intention at some point.

@sspaink sspaink added the help wanted Request for community participation, code, contribution label Apr 25, 2022
@sspaink sspaink added the size/l 1 week or more effort label Aug 2, 2022
@srebhan srebhan self-assigned this Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unexpected problem or unintended behavior help wanted Request for community participation, code, contribution size/l 1 week or more effort
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants