encoding/csv: parser does not preserve \r\n line ending

[ghost]

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8.1 linux/amd64

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/alexhultman/go"
GORACE=""
GOROOT="/usr/lib/golang"
GOTOOLDIR="/usr/lib/golang/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build738528540=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"

What did you do?

Read CSV file using standard CSV parser.

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

Preserved line endings aka I want the verbatim data as in the CSV file, not some made up corrupt data.

What did you see instead?

\r\n is read as \n

[ALTree]

This report is unclear. Please post an auto-contained code snippet that exhibits the issue.

[ghost]

package main

import (
	"encoding/csv"
	"encoding/json"
	"fmt"
	"io"
	"log"
	"strings"
)

func main() {
	in := "\"Hello\r\nHi\""
	r := csv.NewReader(strings.NewReader(in))

	for {
		record, err := r.Read()
		if err == io.EOF {
			break
		}
		if err != nil {
			log.Fatal(err)
		}

		json, _ := json.Marshal(record[0])
		fmt.Println(string(json))
	}
}

Output:

"Hello\nHi"

[nussjustin]

The function for reading the input explains this behaviour:

// Handle \r\n here. We make the simplifying assumption that
// anytime \r is followed by \n that it can be folded to \n.
// We will not detect files which contain both \r\n and bare \n.

I agree that this is unexpected (and not explicitly tested). It could easily be fixed by not using (*Reader).readRune in the Quoted case of (*Reader).parseField.

I think this is worth fixing.

/cc @pborman Any opinion on this?

[pborman]

Yes, it should use r1, _, err = r.r.ReadRune() inside of quotes.

[nussjustin]

The code must not forget to increment r.column. I also think a new method (readRawRune, or something similiar) would make it more clear that the carriage return is kept explicitly.

In any case, if nobody else wants to tackle this, I can send a patch once the tree opens again.

[ajstarks]

See also: #7897

[nussjustin]

We all agree that the corruption you saw is obviously a bug and should be fixed. Bugs happen and, as I said, I will happily mail a fix for this issue.

One of the problems with CSV is that many people don't care about following the specification. This leads to many different and broken implementations of CSV (e.g. the popular "just join the fields with a comma"-implementation) and CSV files that are hard to parse correctly. The encoding/csv already tries to support some common cases features (\n and \r\n for line endings, non double-quoted quotes in quoted fields, trailing commas, comments, different delimiters) and in trying to do so introduced the bug you found.

Please note that #7879 is really a feature request and not a bug report like this issue. The RFC doesn't handle \r line separators other than \r\n. Yes, it could easily be implemented and encoding/csv already supports just \n, but just because it's relatively easy doesn't mean it's without costs. More features mean more API, more code, more complexity, and more chances for bugs.

We should all strive to keep the language, the stdlib and the rest of the ecosystem simple, while not giving up on user experience. Adding new features to the stdlib, especially without any kind of evidence that they are really needed, should always be avoided if possible. Issue #7879 for example only provides a single example for the requested feature and no further evidence that it is necessary, which makes it really unlinkely that the Go team will accept it, since they have to rely on mostly experience instead.

In my experience most CSV files use either \n or \r\n as line delimiters, so just supporting these two should already handle most cases. But maybe I'm wrong and having different row separators is something common in CSV files, which could make it worth implementing. I can't find any information about this in #7879 and a quick google search didn't yield any evidence that this is the case. I'll gladly let someone convince me that I'm wrong.

If you think that the Go team made the wrong decision, for example with #7897, you can always try to collect more examples/evidence and ask them to rethink their decision or try to get opinions from the community. Not everything the Go team and us other contributors do is always perfect, and I think everyone here is old enough to understand this. Just keep in mind that making everyone happy is not always feasible, not just technically but also in terms of complexity.

I could write much more about this, but we should try to keep this about the issue itself.

[pborman]

@alexhultman I hope you realize that we agreed that you had reported a bug an it should be fixed. You have pointed out a situation in which the CSV package is not following the RFC. There have been other changes to the CSV parser that have been accepted since I initially wrote it in 2011. @nussjustin is correct that there are so many different flavors of CSV files. At that time, RFC 4180 was the only standard from a standards body I could find that discussed the CSV format.

@nussjustin if you want to provide the fix, I would be all for that. And you are absolutely correct about trying to keep correct track of the cursor while walking through the input. If you could address #19019 at the same time that would be great (since it is also related to quotes).

[gopherbot]

Change https://golang.org/cl/52810 mentions this issue: encoding/csv: preserve \r\n in quoted fields

[ghost]

thanks

[rsc]

I disagree that this is a problem. I filed #22746 to revert the CL for Go 1.10. If you think the CL should be kept, please comment there. Thanks.

[gopherbot]

Change https://golang.org/cl/78295 mentions this issue: encoding/csv: restore Go 1.9 quoted \r\n handling in Reader