quic: fix panic when handling resent CRYPTO data

When pipe.discardBefore was called with an offset greater
than the current pipe.end position, we would update
pipe.start but not pipe.end, leaving the pipe in an
inconsistent state where start > end. This could then
subsequently cause a panic when writing data that
lies before pipe.start.

This sequence occurs when handling several in-order
CRYPTO frames (where we skip writing in-order data
to the pipe, but still call discardBefore), followed
by an out-of-order frame containing resent data.

Change-Id: Ibac0caad53cd30dac1cd4719a825226809872d96
Reviewed-on: https://go-review.googlesource.com/c/net/+/538775
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>

Author: neild

internal/quic/crypto_stream_test.go

@@ -94,6 +94,21 @@ func TestCryptoStreamReceive(t *testing.T) {
 			end:   3000,
 			want:  4000,
 		}},
+	}, {
+		name: "resent consumed data",
+		frames: []frame{{
+			start: 0,
+			end:   1000,
+			want:  1000,
+		}, {
+			start: 1000,
+			end:   2000,
+			want:  2000,
+		}, {
+			start: 0,
+			end:   1000,
+			want:  2000,
+		}},
 	}} {
 		t.Run(test.name, func(t *testing.T) {
 			var s cryptoStream

internal/quic/pipe.go

@@ -146,4 +146,5 @@ func (p *pipe) discardBefore(off int64) {
 		p.tail = nil
 	}
 	p.start = off
+	p.end = max(p.end, off)
 }

internal/quic/pipe_test.go

@@ -61,6 +61,12 @@ func TestPipeWrites(t *testing.T) {
 			discardBeforeOp{10000},
 			writeOp{10000, 20000},
 		},
+	}, {
+		desc: "discard before writing",
+		ops: []op{
+			discardBeforeOp{1000},
+			writeOp{0, 1},
+		},
 	}} {
 		var p pipe
 		var wantset rangeset[int64]
@@ -78,6 +84,9 @@ func TestPipeWrites(t *testing.T) {
 				p.discardBefore(o.off)
 				wantset.sub(0, o.off)
 				wantStart = o.off
+				if o.off > wantEnd {
+					wantEnd = o.off
+				}
 			}
 			if p.start != wantStart || p.end != wantEnd {
 				t.Errorf("%v: after %#v p contains [%v,%v), want [%v,%v)", test.desc, test.ops[:i+1], p.start, p.end, wantStart, wantEnd)