Support CRLF endings

[owlscode]

Witnessed behavior

go-yaml does not support CRLF line endings when reading a file directly (which would be the main use case). For instance, the following code won't work if the loaded file is written on Windows:

type Auth struct {
	Customer struct {
		Key    string
		Secret string
	}
	App struct {
		Key    string
		Secret string
	}
}

func main() {
	data, err := ioutil.ReadFile("./secret.yml")
	if err != nil {
		fmt.Println(err.Error())
		os.Exit(1)
	}

	a := Auth{}
	err = yaml.Unmarshal(data, &a)
	if err != nil {
		fmt.Println(err.Error())
		os.Exit(1)
	}
	fmt.Println(a)
}

With the following file content:

--- 
app:
  key: 10475405799b09120-vSkxPPI9HsWN61vO4DroQUGFpgNCs2
  secret: Z6nG4UInQVWx7Sagpmu6GsUYWZsvd5E6uIbPdlW8csfemaxm
customer:
  key: JA2PXLd8qeXTDpx7UccWsvp60V6
  secret: qOsUM6996LQSkRw9HsnxyU3EVJzACIOEUxCBTiOcTELTF800VVQ

Here's the error: yaml: line 3: mapping values are not allowed in this context

Expected behavior

Just work as usual.

[mycrEEpy]

same problem here with the following code and gopkg.in/yaml.v3:

	r, err := os.Open(configPath)
	if err != nil {
		return nil, err
	}
	defer r.Close()
	decoder := yaml.NewDecoder(r)
	config := Config{}
	err := decoder.Decode(&config)
	if err != nil {
		return nil, err
	}

[ShimiT]

we are having the same issue on windows machines when using V3 version.

[andreynering]

Hi @niemeyer,

Thanks for your work on this library, and your care for the OSS. ❤️

I consider this bug to be critical, since it makes this package unusable for Windows users. This is preventing us to adopt v3 (go-task/task#201).

[itscaro]

@niemeyer I've juste upgraded from v2 to v3 and encountered this issue.

[niemeyer]

Thanks for the reports. I'll get that fixed.

[niemeyer]

I'm unable to reproduce the error locally, even if I take out the commits from the last few months one by one.

Can someone that is being affected by this problem please provide a self-contained reproducer that does not involve reading an external file? Maybe I'm missing something about how that file really looks like.

[itscaro]

@niemeyer just tested, it seems to be ok with the latest commit of v3 branch 117fdf03f45fe04c0817b3f5426361c2f531f92d, the commit I have issue is this one 55513cacd4ae8b250e3a9084ab9d8c407b1ed618

https://github.com/itscaro/go-yaml-bug-crlf

The error varies in fonction of the yaml.

root@d25edfa03970:/app# cat go.mod
module github.com/itscaro/go-yaml-bug-crlf

require gopkg.in/yaml.v3 v3.0.0-20190502103701-55513cacd4ae
root@d25edfa03970:/app# go run main.go
map[test1:1 test2:2 test3:[a b]]
%!s(<nil>)
-----
%!s(<nil>)
yaml: line 1: did not find expected key

[andreynering]

@itscaro

just tested, it seems to be ok with the master

That's because master is still on v2, AFAIK.

[rfay]

This file has the CRLFs in it that will cause the problem. It's un-parseable by yaml v3.

[itscaro]

@rfay did you try with the latest commit of v3 branch?

@andreynering The tests I did were with v3 branch, not master, initially I thought it was master :) sorry for the misinfo

* commit 117fdf03f45fe04c0817b3f5426361c2f531f92d (HEAD -> v3, origin/v3) with this commit CRLF is not anymore an issue.

[rfay]

No, I backed v3 out completely, had this and other trouble with v3, so the value's not there. However, I added a test in our app for this situation. It will be a while before we try v3 again. Upgraded to it for the hope of comment-preservation, but it seems that isn't mature yet.

I definitely don't see anything mentioning this issue in 117fdf0 and don't see any tests for the CRLF problem either, or a mention of that anywhere.

Is it really testing on go 1.4??? Really? And only one test? https://travis-ci.org/go-yaml/yaml/jobs/554663638 And only on one OS?

[itscaro]

@niemeyer I've just updated my repo the test is self-contained

https://github.com/itscaro/go-yaml-bug-crlf

55513ca...117fdf0

go-yaml-bug-crlf master *% > $ go run .\main.go
go: finding github.com/kr/pretty v0.1.0
go: finding gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127
go: finding gopkg.in/yaml.v3 v3.0.0-20190502103701-55513cacd4ae
go: finding github.com/kr/text v0.1.0
go: finding gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405
go: finding github.com/kr/pty v1.1.1
go: downloading gopkg.in/yaml.v3 v3.0.0-20190502103701-55513cacd4ae
go: extracting gopkg.in/yaml.v3 v3.0.0-20190502103701-55513cacd4ae
map[test1:1 test2:2 test3:[a b]]
%!s(<nil>)
-----
%!s(<nil>)
yaml: line 1: did not find expected key



go-yaml-bug-crlf master *% > $ go run .\main.go
go: finding gopkg.in/yaml.v3 v3.0.0-20190705120443-117fdf03f45f
go: downloading gopkg.in/yaml.v3 v3.0.0-20190705120443-117fdf03f45f
go: extracting gopkg.in/yaml.v3 v3.0.0-20190705120443-117fdf03f45f
map[test1:1 test2:2 test3:[a b]]
%!s(<nil>)
-----
map[test1:1 test2:2 test3:[a b]]
%!s(<nil>)

[andreynering]

Hey @rfay, let's be more kind to maintainers, they're doing free work for you (and the community).

I think Travis is probably running all tests, but not printing the entire output. Something to check, though...

[niemeyer]

@rfay That's not how Travis works.. what you're looking at is a job, not the build. If you want to see all 10 Go versions being tested (1.4 to 1.12 plus tip) you need to click on that "Build" link which is the parent of that job.

@itscaro Thanks for the test case. I've just run it with latest v3 and it works fine.

I've also added a trivial test with 674ba3e just in case.

So considering this a non-issue.

[niemeyer]

Seems a bit awkward, by the way. So much misdirected excitement above for a non-issue. If someone has a self-contained test case they'd like me to look into, please feel free to comment here still.

[andreynering]

Hi @niemeyer,

Seems a bit awkward, by the way. So much misdirected excitement above for a non-issue.

Just making it clear that this was indeed a real issue until 117fdf0. I just tested it: on the previous commit, it fails, but with this one it's somehow fixed.

So while I agree with your comment above, I kindly disagree this was a non-issue.

Sorry for the noise but thanks for your patience. 🙂