bufio, bytes, strings: handle negative runes in WriteRune

Updates #43254

Change-Id: I7d4bf3b99cc36ca2156af5bb01a1c595419d1d3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/280492
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Rob Pike <r@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
TryBot-Result: Go Bot <gobot@golang.org>

diff --git a/src/bufio/bufio.go b/src/bufio/bufio.go
index 6baf9b9e400f388c621bd8f5709d8918f1feebff..ec928e7ad69ed54d213d3a38d9efb011ff32eb82 100644 (file)
--- a/src/bufio/bufio.go
+++ b/src/bufio/bufio.go
@@ -670,7 +670,8 @@ func (b *Writer) WriteByte(c byte) error {
 // WriteRune writes a single Unicode code point, returning
 // the number of bytes written and any error.
 func (b *Writer) WriteRune(r rune) (size int, err error) {
-       if r < utf8.RuneSelf {
+       // Compare as uint32 to correctly handle negative runes.
+       if uint32(r) < utf8.RuneSelf {
                err = b.WriteByte(byte(r))
                if err != nil {
                        return 0, err

diff --git a/src/bufio/bufio_test.go b/src/bufio/bufio_test.go
index d7b34bd0d8f66b485a49ad355fce86c3a9d5f726..ebcc711db9d48cdefa44d6ec9bddd2406c13b59d 100644 (file)
--- a/src/bufio/bufio_test.go
+++ b/src/bufio/bufio_test.go
@@ -534,6 +534,20 @@ func TestReadWriteRune(t *testing.T) {
        }
 }
 
+func TestWriteInvalidRune(t *testing.T) {
+       // Invalid runes, including negative ones, should be written as the
+       // replacement character.
+       for _, r := range []rune{-1, utf8.MaxRune + 1} {
+               var buf bytes.Buffer
+               w := NewWriter(&buf)
+               w.WriteRune(r)
+               w.Flush()
+               if s := buf.String(); s != "\uFFFD" {
+                       t.Errorf("WriteRune(%d) wrote %q, not replacement character", r, s)
+               }
+       }
+}
+
 func TestReadStringAllocs(t *testing.T) {
        r := strings.NewReader("       foo       foo        42        42        42        42        42        42        42        42       4.2       4.2       4.2       4.2\n")
        buf := NewReader(r)

diff --git a/src/bytes/buffer.go b/src/bytes/buffer.go
index f19a4cfff0926c7eba61cead788062b074b1a091..549b077708f80e3742e79b0e93e29d3432d3c6c7 100644 (file)
--- a/src/bytes/buffer.go
+++ b/src/bytes/buffer.go
@@ -275,7 +275,8 @@ func (b *Buffer) WriteByte(c byte) error {
 // included to match bufio.Writer's WriteRune. The buffer is grown as needed;
 // if it becomes too large, WriteRune will panic with ErrTooLarge.
 func (b *Buffer) WriteRune(r rune) (n int, err error) {
-       if r < utf8.RuneSelf {
+       // Compare as uint32 to correctly handle negative runes.
+       if uint32(r) < utf8.RuneSelf {
                b.WriteByte(byte(r))
                return 1, nil
        }

diff --git a/src/bytes/buffer_test.go b/src/bytes/buffer_test.go
index fec5ef8a35f637dacd1c95b9ad3034b6f3559308..9c9b7440ffaa753625a365534a2c7ec30baf4ff2 100644 (file)
--- a/src/bytes/buffer_test.go
+++ b/src/bytes/buffer_test.go
@@ -6,6 +6,7 @@ package bytes_test
 
 import (
        . "bytes"
+       "fmt"
        "io"
        "math/rand"
        "testing"
@@ -387,6 +388,16 @@ func TestRuneIO(t *testing.T) {
        }
 }
 
+func TestWriteInvalidRune(t *testing.T) {
+       // Invalid runes, including negative ones, should be written as
+       // utf8.RuneError.
+       for _, r := range []rune{-1, utf8.MaxRune + 1} {
+               var buf Buffer
+               buf.WriteRune(r)
+               check(t, fmt.Sprintf("TestWriteInvalidRune (%d)", r), &buf, "\uFFFD")
+       }
+}
+
 func TestNext(t *testing.T) {
        b := []byte{0, 1, 2, 3, 4}
        tmp := make([]byte, 5)

diff --git a/src/strings/builder.go b/src/strings/builder.go
index 6ff151d74b2e87e1be45ad178b00383f4e9891bc..547e52e84dde0340e173f9e34b0c54f545c15d48 100644 (file)
--- a/src/strings/builder.go
+++ b/src/strings/builder.go
@@ -103,7 +103,8 @@ func (b *Builder) WriteByte(c byte) error {
 // It returns the length of r and a nil error.
 func (b *Builder) WriteRune(r rune) (int, error) {
        b.copyCheck()
-       if r < utf8.RuneSelf {
+       // Compare as uint32 to correctly handle negative runes.
+       if uint32(r) < utf8.RuneSelf {
                b.buf = append(b.buf, byte(r))
                return 1, nil
        }

diff --git a/src/strings/builder_test.go b/src/strings/builder_test.go
index b662efe7a57d8ceb9f5241e371ca9a70c520db95..e3d239266fd959aa9e4a4f38ec8cec7d43a0e506 100644 (file)
--- a/src/strings/builder_test.go
+++ b/src/strings/builder_test.go
@@ -8,6 +8,7 @@ import (
        "bytes"
        . "strings"
        "testing"
+       "unicode/utf8"
 )
 
 func check(t *testing.T, b *Builder, want string) {
@@ -301,6 +302,16 @@ func TestBuilderCopyPanic(t *testing.T) {
        }
 }
 
+func TestBuilderWriteInvalidRune(t *testing.T) {
+       // Invalid runes, including negative ones, should be written as
+       // utf8.RuneError.
+       for _, r := range []rune{-1, utf8.MaxRune + 1} {
+               var b Builder
+               b.WriteRune(r)
+               check(t, &b, "\uFFFD")
+       }
+}
+
 var someBytes = []byte("some bytes sdljlk jsklj3lkjlk djlkjw")
 
 var sinkS string