quic: fix buffer slicing when handling overlapping stream data

When processing a STREAM frame that overlaps with already received data,
our slicing calculation is wrong. As a result, upon receiving duplicate
data, such as when packets are retransmitted, we end up writing the same
data multiple times rather than deduplicating them. This seems to have
resulted in spurious FINAL_SIZE_ERROR in our tests.

For golang/go#78737

Change-Id: I0c6d34d7f7f376310dad43983cbdf32b6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/net/+/776080
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>

Author: nicholashusin

quic/stream.go

@@ -815,7 +815,7 @@ func (s *Stream) handleData(off int64, b []byte, fin bool) error {
 		// sending us different data than we received the first time.
 		// We currently don't bother.)
 		newOff := min(end, s.inset[0].end)
-		b = b[end-newOff:]
+		b = b[newOff-off:]
 		off = newOff
 	}
 	s.in.writeAt(b, off)

quic/stream_test.go

@@ -395,48 +395,70 @@ func TestStreamReceive(t *testing.T) {
 			wantEOF: true,
 		}},
 	}} {
-		testStreamTypesSynctest(t, test.name, func(t *testing.T, styp streamType) {
-			tc := newTestConn(t, serverSide)
-			tc.handshake()
-			sid := newStreamID(clientSide, styp, 0)
-			var s *Stream
-			got := make([]byte, len(want))
-			var total int
-			for _, f := range test.frames {
-				t.Logf("receive [%v,%v)", f.start, f.end)
-				tc.writeFrames(packetType1RTT, debugFrameStream{
-					id:   sid,
-					off:  f.start,
-					data: want[f.start:f.end],
-					fin:  f.fin,
-				})
-				if s == nil {
-					s = tc.acceptStream()
-				}
-				for {
-					n, err := s.Read(got[total:])
-					t.Logf("s.Read() = %v, %v", n, err)
-					total += n
-					if f.wantEOF && err != io.EOF {
-						t.Fatalf("Read() error = %v; want io.EOF", err)
+		for _, buffered := range []bool{true, false} {
+			name := test.name
+			if buffered {
+				name += "/buffered"
+			} else {
+				name += "/unbuffered"
+			}
+			testStreamTypesSynctest(t, name, func(t *testing.T, styp streamType) {
+				tc := newTestConn(t, serverSide)
+				tc.handshake()
+				sid := newStreamID(clientSide, styp, 0)
+				var s *Stream
+				got := make([]byte, len(want))
+				var total int
+
+				// readAndVerify reads all the data that has been buffered in s
+				// so far, and verifies whether it contains the expected bytes
+				// and EOF.
+				readAndVerify := func(wantEOF bool, wantTotal int) {
+					for {
+						n, err := s.Read(got[total:])
+						t.Logf("s.Read() = %v, %v", n, err)
+						total += n
+						if wantEOF && err != io.EOF {
+							t.Fatalf("Read() error = %v; want io.EOF", err)
+						}
+						if !wantEOF && err == io.EOF {
+							t.Fatalf("Read() error = io.EOF, want something else")
+						}
+						if err != nil {
+							break
+						}
 					}
-					if !f.wantEOF && err == io.EOF {
-						t.Fatalf("Read() error = io.EOF, want something else")
+					if total != wantTotal {
+						t.Fatalf("total bytes read = %v, want %v", total, wantTotal)
 					}
-					if err != nil {
-						break
+					for i := 0; i < total; i++ {
+						if got[i] != want[i] {
+							t.Fatalf("byte %v differs: got %v, want %v", i, got[i], want[i])
+						}
 					}
 				}
-				if total != f.want {
-					t.Fatalf("total bytes read = %v, want %v", total, f.want)
-				}
-				for i := 0; i < total; i++ {
-					if got[i] != want[i] {
-						t.Fatalf("byte %v differs: got %v, want %v", i, got[i], want[i])
+
+				for _, f := range test.frames {
+					t.Logf("receive [%v,%v)", f.start, f.end)
+					tc.writeFrames(packetType1RTT, debugFrameStream{
+						id:   sid,
+						off:  f.start,
+						data: want[f.start:f.end],
+						fin:  f.fin,
+					})
+					if s == nil {
+						s = tc.acceptStream()
+					}
+					if !buffered {
+						readAndVerify(f.wantEOF, f.want)
 					}
 				}
-			}
-		})
+				if buffered {
+					finalFrame := test.frames[len(test.frames)-1]
+					readAndVerify(finalFrame.wantEOF, finalFrame.want)
+				}
+			})
+		}
 	}
 
 }