From: Joe Tsai Date: Sat, 16 Jul 2016 09:42:52 +0000 (-0700) Subject: fmt: properly handle early io.EOF Reads in readRune.readByte X-Git-Tag: go1.7rc3~1^2~15 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=510fb6397dfc93067dc90d42c58dfc5f8b995285;p=gostls13.git fmt: properly handle early io.EOF Reads in readRune.readByte Change https://golang.org/cl/19895 caused a regression where the last character in a string would be dropped if it was accompanied by an io.EOF. This change fixes the logic so that the last byte is still returned without a problem. Fixes #16393 Change-Id: I7a4d0abf761c2c15454136a79e065fe002d736ea Reviewed-on: https://go-review.googlesource.com/24981 Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- diff --git a/src/fmt/scan.go b/src/fmt/scan.go index 08b0bf96a6..fdf419795d 100644 --- a/src/fmt/scan.go +++ b/src/fmt/scan.go @@ -325,9 +325,9 @@ func (r *readRune) readByte() (b byte, err error) { r.pending-- return } - _, err = r.reader.Read(r.pendBuf[:1]) - if err != nil { - return + n, err := io.ReadFull(r.reader, r.pendBuf[:1]) + if n != 1 { + return 0, err } return r.pendBuf[0], err } diff --git a/src/fmt/scan_test.go b/src/fmt/scan_test.go index 364d4fb42a..e36b62e78a 100644 --- a/src/fmt/scan_test.go +++ b/src/fmt/scan_test.go @@ -15,6 +15,7 @@ import ( "regexp" "strings" "testing" + "testing/iotest" "unicode/utf8" ) @@ -118,20 +119,6 @@ func (s *IntString) Scan(state ScanState, verb rune) error { var intStringVal IntString -// myStringReader implements Read but not ReadRune, allowing us to test our readRune wrapper -// type that creates something that can read runes given only Read(). -type myStringReader struct { - r *strings.Reader -} - -func (s *myStringReader) Read(p []byte) (n int, err error) { - return s.r.Read(p) -} - -func newReader(s string) *myStringReader { - return &myStringReader{strings.NewReader(s)} -} - var scanTests = []ScanTest{ // Basic types {"T\n", &boolVal, true}, // boolean test vals toggle to be sure they are written @@ -363,25 +350,38 @@ var multiTests = []ScanfMultiTest{ {"%v%v", "FALSE23", args(&truth, &i), args(false, 23), ""}, } -func testScan(name string, t *testing.T, scan func(r io.Reader, a ...interface{}) (int, error)) { +var readers = []struct { + name string + f func(string) io.Reader +}{ + {"StringReader", func(s string) io.Reader { + return strings.NewReader(s) + }}, + {"ReaderOnly", func(s string) io.Reader { + return struct{ io.Reader }{strings.NewReader(s)} + }}, + {"OneByteReader", func(s string) io.Reader { + return iotest.OneByteReader(strings.NewReader(s)) + }}, + {"DataErrReader", func(s string) io.Reader { + return iotest.DataErrReader(strings.NewReader(s)) + }}, +} + +func testScan(t *testing.T, f func(string) io.Reader, scan func(r io.Reader, a ...interface{}) (int, error)) { for _, test := range scanTests { - var r io.Reader - if name == "StringReader" { - r = strings.NewReader(test.text) - } else { - r = newReader(test.text) - } + r := f(test.text) n, err := scan(r, test.in) if err != nil { m := "" if n > 0 { m = Sprintf(" (%d fields ok)", n) } - t.Errorf("%s got error scanning %q: %s%s", name, test.text, err, m) + t.Errorf("got error scanning %q: %s%s", test.text, err, m) continue } if n != 1 { - t.Errorf("%s count error on entry %q: got %d", name, test.text, n) + t.Errorf("count error on entry %q: got %d", test.text, n) continue } // The incoming value may be a pointer @@ -391,25 +391,25 @@ func testScan(name string, t *testing.T, scan func(r io.Reader, a ...interface{} } val := v.Interface() if !reflect.DeepEqual(val, test.out) { - t.Errorf("%s scanning %q: expected %#v got %#v, type %T", name, test.text, test.out, val, val) + t.Errorf("scanning %q: expected %#v got %#v, type %T", test.text, test.out, val, val) } } } func TestScan(t *testing.T) { - testScan("StringReader", t, Fscan) -} - -func TestMyReaderScan(t *testing.T) { - testScan("myStringReader", t, Fscan) + for _, r := range readers { + t.Run(r.name, func(t *testing.T) { + testScan(t, r.f, Fscan) + }) + } } func TestScanln(t *testing.T) { - testScan("StringReader", t, Fscanln) -} - -func TestMyReaderScanln(t *testing.T) { - testScan("myStringReader", t, Fscanln) + for _, r := range readers { + t.Run(r.name, func(t *testing.T) { + testScan(t, r.f, Fscanln) + }) + } } func TestScanf(t *testing.T) { @@ -500,15 +500,10 @@ func TestInf(t *testing.T) { } } -func testScanfMulti(name string, t *testing.T) { +func testScanfMulti(t *testing.T, f func(string) io.Reader) { sliceType := reflect.TypeOf(make([]interface{}, 1)) for _, test := range multiTests { - var r io.Reader - if name == "StringReader" { - r = strings.NewReader(test.text) - } else { - r = newReader(test.text) - } + r := f(test.text) n, err := Fscanf(r, test.format, test.in...) if err != nil { if test.err == "" { @@ -539,11 +534,11 @@ func testScanfMulti(name string, t *testing.T) { } func TestScanfMulti(t *testing.T) { - testScanfMulti("StringReader", t) -} - -func TestMyReaderScanfMulti(t *testing.T) { - testScanfMulti("myStringReader", t) + for _, r := range readers { + t.Run(r.name, func(t *testing.T) { + testScanfMulti(t, r.f) + }) + } } func TestScanMultiple(t *testing.T) { @@ -818,20 +813,10 @@ func TestMultiLine(t *testing.T) { } } -// simpleReader is a strings.Reader that implements only Read, not ReadRune. -// Good for testing readahead. -type simpleReader struct { - sr *strings.Reader -} - -func (s *simpleReader) Read(b []byte) (n int, err error) { - return s.sr.Read(b) -} - // TestLineByLineFscanf tests that Fscanf does not read past newline. Issue // 3481. func TestLineByLineFscanf(t *testing.T) { - r := &simpleReader{strings.NewReader("1\n2\n")} + r := struct{ io.Reader }{strings.NewReader("1\n2\n")} var i, j int n, err := Fscanf(r, "%v\n", &i) if n != 1 || err != nil { @@ -1000,7 +985,7 @@ func BenchmarkScanRecursiveIntReaderWrapper(b *testing.B) { ints := makeInts(intCount) var r RecursiveInt for i := b.N - 1; i >= 0; i-- { - buf := newReader(string(ints)) + buf := struct{ io.Reader }{strings.NewReader(string(ints))} b.StartTimer() Fscan(buf, &r) b.StopTimer()