From 0b9d805e1ac128e68029335ba96fadf812cd4be5 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 22 Jan 2018 09:51:28 +1100 Subject: [PATCH] correctly handle completed streams when popping frames The streamsMap returns a nil stream (and a nil error) when the stream has already completed. --- stream_framer.go | 5 ++++- stream_framer_test.go | 3 +-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/stream_framer.go b/stream_framer.go index c66815cd..5b89deb8 100644 --- a/stream_framer.go +++ b/stream_framer.go @@ -113,8 +113,11 @@ func (f *streamFramer) maybePopNormalFrames(maxTotalLen protocol.ByteCount) []*w } id := f.streamQueue[0] f.streamQueue = f.streamQueue[1:] + // This should never return an error. Better check it anyway. + // The stream will only be in the streamQueue, if it enqueued itself there. str, err := f.streamGetter.GetOrOpenSendStream(id) - if err != nil { // can happen if the stream completed after it said it had data + // The stream can be nil if it completed after it said it had data. + if str == nil || err != nil { delete(f.activeStreams, id) continue } diff --git a/stream_framer_test.go b/stream_framer_test.go index 1a4002f2..8403dba2 100644 --- a/stream_framer_test.go +++ b/stream_framer_test.go @@ -2,7 +2,6 @@ package quic import ( "bytes" - "errors" "github.com/golang/mock/gomock" @@ -139,7 +138,7 @@ var _ = Describe("Stream Framer", func() { }) It("skips a stream that was reported active, but was completed shortly after", func() { - streamGetter.EXPECT().GetOrOpenSendStream(id1).Return(nil, errors.New("stream was already deleted")) + streamGetter.EXPECT().GetOrOpenSendStream(id1).Return(nil, nil) streamGetter.EXPECT().GetOrOpenSendStream(id2).Return(stream2, nil) f := &wire.StreamFrame{ StreamID: id2,