diff --git a/api/go1.10.txt b/api/go1.10.txt index c8e504c992731..6647ec66dc7d1 100644 --- a/api/go1.10.txt +++ b/api/go1.10.txt @@ -594,7 +594,6 @@ pkg os, method (*SyscallError) Timeout() bool pkg os, var ErrNoDeadline error pkg strings, method (*Builder) Grow(int) pkg strings, method (*Builder) Len() int -pkg strings, method (*Builder) ReadFrom(io.Reader) (int64, error) pkg strings, method (*Builder) Reset() pkg strings, method (*Builder) String() string pkg strings, method (*Builder) Write([]uint8) (int, error) diff --git a/src/strings/builder.go b/src/strings/builder.go index 09ebb3d91bd16..11bcee1dfcdfb 100644 --- a/src/strings/builder.go +++ b/src/strings/builder.go @@ -5,16 +5,24 @@ package strings import ( - "errors" - "io" "unicode/utf8" "unsafe" ) // A Builder is used to efficiently build a string using Write methods. // It minimizes memory copying. The zero value is ready to use. +// Do not copy a non-zero Builder. type Builder struct { - buf []byte + addr *Builder // of receiver, to detect copies by value + buf []byte +} + +func (b *Builder) copyCheck() { + if b.addr == nil { + b.addr = b + } else if b.addr != b { + panic("strings: illegal use of non-zero Builder copied by value") + } } // String returns the accumulated string. @@ -26,7 +34,10 @@ func (b *Builder) String() string { func (b *Builder) Len() int { return len(b.buf) } // Reset resets the Builder to be empty. -func (b *Builder) Reset() { b.buf = nil } +func (b *Builder) Reset() { + b.addr = nil + b.buf = nil +} // grow copies the buffer to a new, larger buffer so that there are at least n // bytes of capacity beyond len(b.buf). @@ -40,6 +51,7 @@ func (b *Builder) grow(n int) { // another n bytes. After Grow(n), at least n bytes can be written to b // without another allocation. If n is negative, Grow panics. func (b *Builder) Grow(n int) { + b.copyCheck() if n < 0 { panic("strings.Builder.Grow: negative count") } @@ -51,6 +63,7 @@ func (b *Builder) Grow(n int) { // Write appends the contents of p to b's buffer. // Write always returns len(p), nil. func (b *Builder) Write(p []byte) (int, error) { + b.copyCheck() b.buf = append(b.buf, p...) return len(p), nil } @@ -58,6 +71,7 @@ func (b *Builder) Write(p []byte) (int, error) { // WriteByte appends the byte c to b's buffer. // The returned error is always nil. func (b *Builder) WriteByte(c byte) error { + b.copyCheck() b.buf = append(b.buf, c) return nil } @@ -65,6 +79,7 @@ func (b *Builder) WriteByte(c byte) error { // WriteRune appends the UTF-8 encoding of Unicode code point r to b's buffer. // It returns the length of r and a nil error. func (b *Builder) WriteRune(r rune) (int, error) { + b.copyCheck() if r < utf8.RuneSelf { b.buf = append(b.buf, byte(r)) return 1, nil @@ -81,38 +96,7 @@ func (b *Builder) WriteRune(r rune) (int, error) { // WriteString appends the contents of s to b's buffer. // It returns the length of s and a nil error. func (b *Builder) WriteString(s string) (int, error) { + b.copyCheck() b.buf = append(b.buf, s...) return len(s), nil } - -// minRead is the minimum slice passed to a Read call by Builder.ReadFrom. -// It is the same as bytes.MinRead. -const minRead = 512 - -// errNegativeRead is the panic value if the reader passed to Builder.ReadFrom -// returns a negative count. -var errNegativeRead = errors.New("strings.Builder: reader returned negative count from Read") - -// ReadFrom reads data from r until EOF and appends it to b's buffer. -// The return value n is the number of bytes read. -// Any error except io.EOF encountered during the read is also returned. -func (b *Builder) ReadFrom(r io.Reader) (n int64, err error) { - for { - l := len(b.buf) - if cap(b.buf)-l < minRead { - b.grow(minRead) - } - m, e := r.Read(b.buf[l:cap(b.buf)]) - if m < 0 { - panic(errNegativeRead) - } - b.buf = b.buf[:l+m] - n += int64(m) - if e == io.EOF { - return n, nil - } - if e != nil { - return n, e - } - } -} diff --git a/src/strings/builder_test.go b/src/strings/builder_test.go index df557082a797c..c0c8fa4130fb1 100644 --- a/src/strings/builder_test.go +++ b/src/strings/builder_test.go @@ -6,12 +6,9 @@ package strings_test import ( "bytes" - "errors" - "io" "runtime" . "strings" "testing" - "testing/iotest" ) func check(t *testing.T, b *Builder, want string) { @@ -169,93 +166,6 @@ func TestBuilderWriteByte(t *testing.T) { check(t, &b, "a\x00") } -func TestBuilderReadFrom(t *testing.T) { - for _, tt := range []struct { - name string - fn func(io.Reader) io.Reader - }{ - {"Reader", func(r io.Reader) io.Reader { return r }}, - {"DataErrReader", iotest.DataErrReader}, - {"OneByteReader", iotest.OneByteReader}, - } { - t.Run(tt.name, func(t *testing.T) { - var b Builder - - r := tt.fn(NewReader("hello")) - n, err := b.ReadFrom(r) - if err != nil { - t.Fatalf("first call: got %s", err) - } - if n != 5 { - t.Errorf("first call: got n=%d; want 5", n) - } - check(t, &b, "hello") - - r = tt.fn(NewReader(" world")) - n, err = b.ReadFrom(r) - if err != nil { - t.Fatalf("first call: got %s", err) - } - if n != 6 { - t.Errorf("first call: got n=%d; want 6", n) - } - check(t, &b, "hello world") - }) - } -} - -var errRead = errors.New("boom") - -// errorReader sends reads to the underlying reader -// but returns errRead instead of io.EOF. -type errorReader struct { - r io.Reader -} - -func (r errorReader) Read(b []byte) (int, error) { - n, err := r.r.Read(b) - if err == io.EOF { - err = errRead - } - return n, err -} - -func TestBuilderReadFromError(t *testing.T) { - var b Builder - r := errorReader{NewReader("hello")} - n, err := b.ReadFrom(r) - if n != 5 { - t.Errorf("got n=%d; want 5", n) - } - if err != errRead { - t.Errorf("got err=%q; want %q", err, errRead) - } - check(t, &b, "hello") -} - -type negativeReader struct{} - -func (r negativeReader) Read([]byte) (int, error) { return -1, nil } - -func TestBuilderReadFromNegativeReader(t *testing.T) { - var b Builder - defer func() { - switch err := recover().(type) { - case nil: - t.Fatal("ReadFrom didn't panic") - case error: - wantErr := "strings.Builder: reader returned negative count from Read" - if err.Error() != wantErr { - t.Fatalf("recovered panic: got %v; want %v", err.Error(), wantErr) - } - default: - t.Fatalf("unexpected panic value: %#v", err) - } - }() - - b.ReadFrom(negativeReader{}) -} - func TestBuilderAllocs(t *testing.T) { var b Builder b.Grow(5) @@ -280,3 +190,103 @@ func numAllocs(fn func()) uint64 { runtime.ReadMemStats(&m2) return m2.Mallocs - m1.Mallocs } + +func TestBuilderCopyPanic(t *testing.T) { + tests := []struct { + name string + fn func() + wantPanic bool + }{ + { + name: "String", + wantPanic: false, + fn: func() { + var a Builder + a.WriteByte('x') + b := a + _ = b.String() // appease vet + }, + }, + { + name: "Len", + wantPanic: false, + fn: func() { + var a Builder + a.WriteByte('x') + b := a + b.Len() + }, + }, + { + name: "Reset", + wantPanic: false, + fn: func() { + var a Builder + a.WriteByte('x') + b := a + b.Reset() + b.WriteByte('y') + }, + }, + { + name: "Write", + wantPanic: true, + fn: func() { + var a Builder + a.Write([]byte("x")) + b := a + b.Write([]byte("y")) + }, + }, + { + name: "WriteByte", + wantPanic: true, + fn: func() { + var a Builder + a.WriteByte('x') + b := a + b.WriteByte('y') + }, + }, + { + name: "WriteString", + wantPanic: true, + fn: func() { + var a Builder + a.WriteString("x") + b := a + b.WriteString("y") + }, + }, + { + name: "WriteRune", + wantPanic: true, + fn: func() { + var a Builder + a.WriteRune('x') + b := a + b.WriteRune('y') + }, + }, + { + name: "Grow", + wantPanic: true, + fn: func() { + var a Builder + a.Grow(1) + b := a + b.Grow(2) + }, + }, + } + for _, tt := range tests { + didPanic := make(chan bool) + go func() { + defer func() { didPanic <- recover() != nil }() + tt.fn() + }() + if got := <-didPanic; got != tt.wantPanic { + t.Errorf("%s: panicked = %v; want %v", tt.name, got, tt.wantPanic) + } + } +}