encoding/gob: follow documented io.EOF semantics

The docs say:

	If the input is at EOF, Decode returns io.EOF and does not modify e.

However, the added test fails:

	--- FAIL: TestDecodePartial (0.00s)
		encoder_test.go:1263: 31/81: expected io.ErrUnexpectedEOF: EOF
		encoder_test.go:1263: 51/81: expected io.ErrUnexpectedEOF: EOF

In particular, the decoder would return io.EOF after reading a valid
message for a type specification, and then hit EOF before reading a data
item message.

Fix that by only allowing a Decode call to return io.EOF if the reader
hits EOF immediately, without successfully reading any message.
Otherwise, hitting EOF is an ErrUnexpectedEOF, like in other cases.

Also fix a net/rpc test that, coincidentally, expected an io.EOF
as an error when feeding bad non-zero data to a gob decoder.
An io.ErrUnexpectedEOF is clearly better in that scenario.

Fixes #48905.

Change-Id: Ied6a0d8ac8377f89646319a18c0380c4f2b09b85
Reviewed-on: https://go-review.googlesource.com/c/go/+/354972
Trust: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/encoding/gob/decoder.go b/src/encoding/gob/decoder.go
index b476aaac93be537f24ef16c72c208bee6538078d..5e4ed5a7d7afb1f7ad55c040386c41a1b12c87fd 100644 (file)
--- a/src/encoding/gob/decoder.go
+++ b/src/encoding/gob/decoder.go
@@ -138,9 +138,17 @@ func (dec *Decoder) nextUint() uint64 {
 // decoded. If this is an interface value, it can be ignored by
 // resetting that buffer.
 func (dec *Decoder) decodeTypeSequence(isInterface bool) typeId {
+       firstMessage := true
        for dec.err == nil {
                if dec.buf.Len() == 0 {
                        if !dec.recvMessage() {
+                               // We can only return io.EOF if the input was empty.
+                               // If we read one or more type spec messages,
+                               // require a data item message to follow.
+                               // If we hit an EOF before that, then give ErrUnexpectedEOF.
+                               if !firstMessage && dec.err == io.EOF {
+                                       dec.err = io.ErrUnexpectedEOF
+                               }
                                break
                        }
                }
@@ -166,6 +174,7 @@ func (dec *Decoder) decodeTypeSequence(isInterface bool) typeId {
                        }
                        dec.nextUint()
                }
+               firstMessage = false
        }
        return -1
 }

diff --git a/src/encoding/gob/encoder_test.go b/src/encoding/gob/encoder_test.go
index 6d50b825735f811fc4d4228fd3b87528f34cf1ed..a358d5bc3009d682528d50afb667294e2691218f 100644 (file)
--- a/src/encoding/gob/encoder_test.go
+++ b/src/encoding/gob/encoder_test.go
@@ -1202,3 +1202,66 @@ func TestMarshalFloatMap(t *testing.T) {
                t.Fatalf("\nEncode: %v\nDecode: %v", want, got)
        }
 }
+
+func TestDecodePartial(t *testing.T) {
+       type T struct {
+               X []int
+               Y string
+       }
+
+       var buf bytes.Buffer
+       t1 := T{X: []int{1, 2, 3}, Y: "foo"}
+       t2 := T{X: []int{4, 5, 6}, Y: "bar"}
+       enc := NewEncoder(&buf)
+
+       t1start := 0
+       if err := enc.Encode(&t1); err != nil {
+               t.Fatal(err)
+       }
+
+       t2start := buf.Len()
+       if err := enc.Encode(&t2); err != nil {
+               t.Fatal(err)
+       }
+
+       data := buf.Bytes()
+       for i := 0; i <= len(data); i++ {
+               bufr := bytes.NewReader(data[:i])
+
+               // Decode both values, stopping at the first error.
+               var t1b, t2b T
+               dec := NewDecoder(bufr)
+               var err error
+               err = dec.Decode(&t1b)
+               if err == nil {
+                       err = dec.Decode(&t2b)
+               }
+
+               switch i {
+               case t1start, t2start:
+                       // Either the first or the second Decode calls had zero input.
+                       if err != io.EOF {
+                               t.Errorf("%d/%d: expected io.EOF: %v", i, len(data), err)
+                       }
+               case len(data):
+                       // We reached the end of the entire input.
+                       if err != nil {
+                               t.Errorf("%d/%d: unexpected error: %v", i, len(data), err)
+                       }
+                       if !reflect.DeepEqual(t1b, t1) {
+                               t.Fatalf("t1 value mismatch: got %v, want %v", t1b, t1)
+                       }
+                       if !reflect.DeepEqual(t2b, t2) {
+                               t.Fatalf("t2 value mismatch: got %v, want %v", t2b, t2)
+                       }
+               default:
+                       // In between, we must see io.ErrUnexpectedEOF.
+                       // The decoder used to erroneously return io.EOF in some cases here,
+                       // such as if the input was cut off right after some type specs,
+                       // but before any value was actually transmitted.
+                       if err != io.ErrUnexpectedEOF {
+                               t.Errorf("%d/%d: expected io.ErrUnexpectedEOF: %v", i, len(data), err)
+                       }
+               }
+       }
+}

diff --git a/src/net/rpc/client_test.go b/src/net/rpc/client_test.go
index 03225e3d0104b5277d8a0af2fea98be725676c02..38a10ce0b3156f25040d35631a9ff497be201e6d 100644 (file)
--- a/src/net/rpc/client_test.go
+++ b/src/net/rpc/client_test.go
@@ -57,8 +57,8 @@ func TestGobError(t *testing.T) {
                if err == nil {
                        t.Fatal("no error")
                }
-               if !strings.Contains(err.(error).Error(), "reading body EOF") {
-                       t.Fatal("expected `reading body EOF', got", err)
+               if !strings.Contains(err.(error).Error(), "reading body unexpected EOF") {
+                       t.Fatal("expected `reading body unexpected EOF', got", err)
                }
        }()
        Register(new(S))