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>

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

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)

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
 	}

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)

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
 	}

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