remove redundant bool return value from sendStream.popStreamFrame (#4828)

This commit is contained in:
Marten Seemann 2024-12-31 13:51:14 +08:00 committed by GitHub
parent f337891fc8
commit 0b9bd3c4de
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 152 additions and 142 deletions

View file

@ -196,7 +196,7 @@ func (f *framer) AppendStreamFrames(frames []ackhandler.StreamFrame, maxLen prot
// Therefore, we can pretend to have more bytes available when popping
// the STREAM frame (which will always have the DataLen set).
remainingLen += protocol.ByteCount(quicvarint.Len(uint64(remainingLen)))
frame, ok, hasMoreData := str.popStreamFrame(remainingLen, v)
frame, hasMoreData := str.popStreamFrame(remainingLen, v)
if hasMoreData { // put the stream back in the queue (at the end)
f.streamQueue.PushBack(id)
} else { // no more data to send. Stream is not active
@ -205,7 +205,7 @@ func (f *framer) AppendStreamFrames(frames []ackhandler.StreamFrame, maxLen prot
// The frame can be "nil"
// * if the stream was canceled after it said it had data
// * the remaining size doesn't allow us to add another STREAM frame
if !ok {
if frame.Frame == nil {
continue
}
frames = append(frames, frame)

View file

@ -176,9 +176,9 @@ func TestFramerAppendStreamFrames(t *testing.T) {
// add two streams
mockCtrl := gomock.NewController(t)
str1 := NewMockSendStreamI(mockCtrl)
str1.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{Frame: f1}, true, true)
str1.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{Frame: f1}, true)
str2 := NewMockSendStreamI(mockCtrl)
str2.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{Frame: f2}, true, false)
str2.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{Frame: f2}, false)
framer.AddActiveStream(str1ID, str1)
framer.AddActiveStream(str1ID, str1) // duplicate calls are ok (they're no-ops)
framer.AddActiveStream(str2ID, str2)
@ -203,7 +203,7 @@ func TestFramerAppendStreamFrames(t *testing.T) {
require.True(t, framer.HasData()) // the stream claimed to have more data...
// ... but it actually doesn't
str1.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{}, false, false)
str1.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{}, false)
fs, length = framer.AppendStreamFrames(nil, protocol.MaxByteCount, protocol.Version1)
require.Empty(t, fs)
require.Zero(t, length)
@ -235,11 +235,13 @@ func TestFramerMinStreamFrameSize(t *testing.T) {
require.Empty(t, frames)
// pop frames of the minimum size
str.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool) {
str.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(
func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool) {
f := &wire.StreamFrame{StreamID: id, DataLenPresent: true}
f.Data = make([]byte, f.MaxDataLen(protocol.MinStreamFrameSize, v))
return ackhandler.StreamFrame{Frame: f}, true, false
})
return ackhandler.StreamFrame{Frame: f}, false
},
)
frames, _ = framer.AppendStreamFrames(nil, protocol.MinStreamFrameSize, protocol.Version1)
require.Len(t, frames, 1)
// unsetting DataLenPresent on the last frame reduced the size slightly beyond the minimum size
@ -258,7 +260,7 @@ func TestFramerMinStreamFrameSizeMultipleStreamFrames(t *testing.T) {
Data: bytes.Repeat([]byte("f"), int(500-protocol.MinStreamFrameSize)),
DataLenPresent: true,
}
str.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{Frame: f}, true, false)
str.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).Return(ackhandler.StreamFrame{Frame: f}, false)
framer.AddActiveStream(id, str)
fs, length := framer.AppendStreamFrames(nil, 500, protocol.Version1)
require.Len(t, fs, 1)
@ -272,15 +274,17 @@ func TestFramerFillPacketOneStream(t *testing.T) {
framer := newFramer()
for i := protocol.MinStreamFrameSize; i < 2000; i++ {
str.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool) {
str.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(
func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool) {
f := &wire.StreamFrame{
StreamID: id,
DataLenPresent: true,
}
f.Data = make([]byte, f.MaxDataLen(size, v))
require.Equal(t, size, f.Length(protocol.Version1))
return ackhandler.StreamFrame{Frame: f}, true, false
})
return ackhandler.StreamFrame{Frame: f}, false
},
)
framer.AddActiveStream(id, str)
frames, _ := framer.AppendStreamFrames(nil, i, protocol.Version1)
require.Len(t, frames, 1)
@ -301,17 +305,21 @@ func TestFramerFillPacketMultipleStreams(t *testing.T) {
framer := newFramer()
for i := 2 * protocol.MinStreamFrameSize; i < 2000; i++ {
stream1.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool) {
stream1.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(
func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool) {
f := &wire.StreamFrame{StreamID: id1, DataLenPresent: true}
f.Data = make([]byte, f.MaxDataLen(protocol.MinStreamFrameSize, v))
return ackhandler.StreamFrame{Frame: f}, true, false
})
stream2.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool) {
return ackhandler.StreamFrame{Frame: f}, false
},
)
stream2.EXPECT().popStreamFrame(gomock.Any(), protocol.Version1).DoAndReturn(
func(size protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool) {
f := &wire.StreamFrame{StreamID: id2, DataLenPresent: true}
f.Data = make([]byte, f.MaxDataLen(size, v))
require.Equal(t, size, f.Length(protocol.Version1))
return ackhandler.StreamFrame{Frame: f}, true, false
})
return ackhandler.StreamFrame{Frame: f}, false
},
)
framer.AddActiveStream(id1, stream1)
framer.AddActiveStream(id2, stream2)
frames, _ := framer.AppendStreamFrames(nil, i, protocol.Version1)

View file

@ -383,13 +383,12 @@ func (c *MockSendStreamIhasDataCall) DoAndReturn(f func() bool) *MockSendStreamI
}
// popStreamFrame mocks base method.
func (m *MockSendStreamI) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool) {
func (m *MockSendStreamI) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "popStreamFrame", maxBytes, v)
ret0, _ := ret[0].(ackhandler.StreamFrame)
ret1, _ := ret[1].(bool)
ret2, _ := ret[2].(bool)
return ret0, ret1, ret2
return ret0, ret1
}
// popStreamFrame indicates an expected call of popStreamFrame.
@ -405,19 +404,19 @@ type MockSendStreamIpopStreamFrameCall struct {
}
// Return rewrite *gomock.Call.Return
func (c *MockSendStreamIpopStreamFrameCall) Return(frame ackhandler.StreamFrame, ok, hasMore bool) *MockSendStreamIpopStreamFrameCall {
c.Call = c.Call.Return(frame, ok, hasMore)
func (c *MockSendStreamIpopStreamFrameCall) Return(frame ackhandler.StreamFrame, hasMore bool) *MockSendStreamIpopStreamFrameCall {
c.Call = c.Call.Return(frame, hasMore)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockSendStreamIpopStreamFrameCall) Do(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool, bool)) *MockSendStreamIpopStreamFrameCall {
func (c *MockSendStreamIpopStreamFrameCall) Do(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool)) *MockSendStreamIpopStreamFrameCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockSendStreamIpopStreamFrameCall) DoAndReturn(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool, bool)) *MockSendStreamIpopStreamFrameCall {
func (c *MockSendStreamIpopStreamFrameCall) DoAndReturn(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool)) *MockSendStreamIpopStreamFrameCall {
c.Call = c.Call.DoAndReturn(f)
return c
}

View file

@ -610,13 +610,12 @@ func (c *MockStreamIhasDataCall) DoAndReturn(f func() bool) *MockStreamIhasDataC
}
// popStreamFrame mocks base method.
func (m *MockStreamI) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool) {
func (m *MockStreamI) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "popStreamFrame", maxBytes, v)
ret0, _ := ret[0].(ackhandler.StreamFrame)
ret1, _ := ret[1].(bool)
ret2, _ := ret[2].(bool)
return ret0, ret1, ret2
return ret0, ret1
}
// popStreamFrame indicates an expected call of popStreamFrame.
@ -632,19 +631,19 @@ type MockStreamIpopStreamFrameCall struct {
}
// Return rewrite *gomock.Call.Return
func (c *MockStreamIpopStreamFrameCall) Return(arg0 ackhandler.StreamFrame, arg1, arg2 bool) *MockStreamIpopStreamFrameCall {
c.Call = c.Call.Return(arg0, arg1, arg2)
func (c *MockStreamIpopStreamFrameCall) Return(arg0 ackhandler.StreamFrame, arg1 bool) *MockStreamIpopStreamFrameCall {
c.Call = c.Call.Return(arg0, arg1)
return c
}
// Do rewrite *gomock.Call.Do
func (c *MockStreamIpopStreamFrameCall) Do(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool, bool)) *MockStreamIpopStreamFrameCall {
func (c *MockStreamIpopStreamFrameCall) Do(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool)) *MockStreamIpopStreamFrameCall {
c.Call = c.Call.Do(f)
return c
}
// DoAndReturn rewrite *gomock.Call.DoAndReturn
func (c *MockStreamIpopStreamFrameCall) DoAndReturn(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool, bool)) *MockStreamIpopStreamFrameCall {
func (c *MockStreamIpopStreamFrameCall) DoAndReturn(f func(protocol.ByteCount, protocol.Version) (ackhandler.StreamFrame, bool)) *MockStreamIpopStreamFrameCall {
c.Call = c.Call.DoAndReturn(f)
return c
}

View file

@ -18,7 +18,7 @@ type sendStreamI interface {
SendStream
handleStopSendingFrame(*wire.StopSendingFrame)
hasData() bool
popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (frame ackhandler.StreamFrame, ok, hasMore bool)
popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (frame ackhandler.StreamFrame, hasMore bool)
closeForShutdown(error)
updateSendWindow(protocol.ByteCount)
}
@ -217,7 +217,7 @@ func (s *sendStream) canBufferStreamFrame() bool {
// popStreamFrame returns the next STREAM frame that is supposed to be sent on this stream
// maxBytes is the maximum length this frame (including frame header) will have.
func (s *sendStream) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (af ackhandler.StreamFrame, ok, hasMore bool) {
func (s *sendStream) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (af ackhandler.StreamFrame, hasMore bool) {
s.mutex.Lock()
f, hasMoreData, queuedControlFrame := s.popNewOrRetransmittedStreamFrame(maxBytes, v)
if f != nil {
@ -229,12 +229,12 @@ func (s *sendStream) popStreamFrame(maxBytes protocol.ByteCount, v protocol.Vers
s.sender.onHasStreamControlFrame(s.streamID, s)
}
if f == nil {
return ackhandler.StreamFrame{}, false, hasMoreData
return ackhandler.StreamFrame{}, hasMoreData
}
return ackhandler.StreamFrame{
Frame: f,
Handler: (*sendStreamAckHandler)(s),
}, true, hasMoreData
}, hasMoreData
}
func (s *sendStream) popNewOrRetransmittedStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (_ *wire.StreamFrame, hasMoreData, queuedControlFrame bool) {

View file

@ -71,8 +71,7 @@ func TestSendStreamWriteData(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(6))
frame, ok, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Data: []byte("foobar"), DataLenPresent: true},
@ -81,8 +80,7 @@ func TestSendStreamWriteData(t *testing.T) {
require.True(t, mockCtrl.Satisfied())
// nothing more to send at this point
_, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
_, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, hasMore)
require.True(t, mockCtrl.Satisfied())
@ -109,8 +107,7 @@ func TestSendStreamWriteData(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(4))
frame, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: 42, Offset: 6, Data: []byte{0xde, 0xad, 0xbe, 0xef}, DataLenPresent: true},
@ -124,18 +121,16 @@ func TestSendStreamWriteData(t *testing.T) {
require.Equal(t, 6, n)
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount).Times(3)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3)).Times(2)
frame, ok, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 10), protocol.Version1)
require.False(t, ok)
frame, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 10), protocol.Version1)
require.Nil(t, frame.Frame)
require.True(t, hasMore)
frame, ok, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 10)+3, protocol.Version1)
require.True(t, ok)
frame, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 10)+3, protocol.Version1)
require.True(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Offset: 10, Data: []byte("foo"), DataLenPresent: true},
frame.Frame,
)
frame, ok, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 13)+3, protocol.Version1)
require.True(t, ok)
frame, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 13)+3, protocol.Version1)
require.False(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Offset: 13, Data: []byte("baz"), DataLenPresent: true},
@ -170,8 +165,8 @@ func TestSendStreamLargeWrites(t *testing.T) {
var offset protocol.ByteCount
const size = 40
for offset+size < protocol.ByteCount(len(data))-protocol.MaxPacketBufferSize {
frame, ok, hasMore := str.popStreamFrame(size+expectedFrameHeaderLen(streamID, offset), protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(size+expectedFrameHeaderLen(streamID, offset), protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, hasMore)
require.Equal(t, offset, frame.Frame.Offset)
require.Equal(t, data[offset:offset+size], frame.Frame.Data)
@ -186,8 +181,8 @@ func TestSendStreamLargeWrites(t *testing.T) {
}
mockSender.EXPECT().onHasStreamData(streamID, str) // from the Close call
frame, ok, hasMore := str.popStreamFrame(size+expectedFrameHeaderLen(streamID, offset), protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(size+expectedFrameHeaderLen(streamID, offset), protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, hasMore)
require.Equal(t, data[offset:offset+size], frame.Frame.Data)
require.Equal(t, offset, frame.Frame.Offset)
@ -199,8 +194,8 @@ func TestSendStreamLargeWrites(t *testing.T) {
t.Fatal("timeout")
}
frame, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.NotNil(t, frame.Frame)
require.False(t, hasMore)
require.Equal(t, data[offset:], frame.Frame.Data)
require.True(t, frame.Frame.Fin)
@ -230,8 +225,8 @@ func TestSendStreamLargeWriteBlocking(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount).Times(2)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3))
frame, ok, hasMoreData := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0)+3, protocol.Version1)
require.True(t, ok)
frame, hasMoreData := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0)+3, protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, hasMoreData)
require.Equal(t, []byte("foo"), frame.Frame.Data)
@ -242,8 +237,8 @@ func TestSendStreamLargeWriteBlocking(t *testing.T) {
}
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3))
frame, ok, hasMoreData = str.popStreamFrame(expectedFrameHeaderLen(streamID, 3)+3, protocol.Version1)
require.True(t, ok)
frame, hasMoreData = str.popStreamFrame(expectedFrameHeaderLen(streamID, 3)+3, protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, hasMoreData)
require.Equal(t, []byte("bar"), frame.Frame.Data)
@ -270,10 +265,12 @@ func TestSendStreamCopyData(t *testing.T) {
require.NoError(t, err)
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(gomock.Any())
frame, ok, _ := str.popStreamFrame(protocol.MaxPacketBufferSize, protocol.Version1)
require.True(t, ok)
frame, _ := str.popStreamFrame(protocol.MaxPacketBufferSize, protocol.Version1)
data[1] = 'e' // modify the data after it has been written
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Data: []byte("foobar"), DataLenPresent: true}, frame.Frame)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Data: []byte("foobar"), DataLenPresent: true},
frame.Frame,
)
}
func TestSendStreamDeadlineInThePast(t *testing.T) {
@ -345,8 +342,8 @@ func TestSendStreamDeadlineRemoval(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(gomock.Any())
frame, ok, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.NotNil(t, frame.Frame)
require.False(t, hasMoreData)
require.Equal(t, []byte("foobar"), frame.Frame.Data)
}
@ -381,8 +378,8 @@ func TestSendStreamDeadlineExtension(t *testing.T) {
t.Fatal("timeout")
}
_, ok, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMoreData)
}
@ -407,15 +404,14 @@ func TestSendStreamClose(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount).Times(2)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3)).Times(2)
frame, ok, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0)+3, protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0)+3, protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Offset: 0, Data: []byte("foo"), DataLenPresent: true}, // no FIN yet
frame.Frame,
)
frame, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Offset: 3, Fin: true, Data: []byte("bar"), DataLenPresent: true},
@ -426,14 +422,14 @@ func TestSendStreamClose(t *testing.T) {
// further calls to Write return an error
_, err = strWithTimeout.Write([]byte("foobar"))
require.ErrorContains(t, err, "write on closed stream 1234")
_, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
// further calls to Close don't do anything
require.NoError(t, str.Close())
_, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
require.True(t, mockCtrl.Satisfied())
}
@ -446,8 +442,7 @@ func TestSendStreamImmediateClose(t *testing.T) {
str := newSendStream(context.Background(), streamID, mockSender, mockFC)
mockSender.EXPECT().onHasStreamData(streamID, str)
require.NoError(t, str.Close())
frame, ok, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 13)+3, protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 13)+3, protocol.Version1)
require.False(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Fin: true, DataLenPresent: true},
@ -470,20 +465,25 @@ func TestSendStreamFlowControlBlocked(t *testing.T) {
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3))
mockFC.EXPECT().SendWindowSize().Return(protocol.ByteCount(0))
mockFC.EXPECT().IsNewlyBlocked().Return(true)
frame, ok, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, hasMore)
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true}, frame.Frame)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true},
frame.Frame,
)
// TODO(#4771): the STREAM_DATA_BLOCKED frame should be sent immediately
mockSender.EXPECT().onHasStreamControlFrame(streamID, str)
frame, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
cf, ok, hasMore := str.getControlFrame(time.Now())
require.True(t, ok)
require.EqualExportedValues(t, &wire.StreamDataBlockedFrame{StreamID: streamID, MaximumStreamData: 3}, cf.Frame)
require.EqualExportedValues(t,
&wire.StreamDataBlockedFrame{StreamID: streamID, MaximumStreamData: 3},
cf.Frame,
)
require.False(t, hasMore)
}
@ -527,8 +527,8 @@ func TestSendStreamCloseForShutdown(t *testing.T) {
require.NoError(t, str.Close())
// no STREAM frames popped
_, ok, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
// canceling the stream doesn't do anything
@ -566,8 +566,8 @@ func TestSendStreamCancellation(t *testing.T) {
require.NoError(t, err)
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3))
frame, ok, hasMore := str.popStreamFrame(3+expectedFrameHeaderLen(streamID, 0), protocol.Version1)
require.True(t, ok)
frame, hasMore := str.popStreamFrame(3+expectedFrameHeaderLen(streamID, 0), protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, hasMore)
require.Equal(t, []byte("foo"), frame.Frame.Data)
require.True(t, mockCtrl.Satisfied())
@ -619,21 +619,21 @@ func TestSendStreamCancellation(t *testing.T) {
}
// no data to send
_, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
// future calls to Write should return an error
_, err = strWithTimeout.Write([]byte("foo"))
require.ErrorIs(t, err, &StreamError{StreamID: streamID, ErrorCode: 1234, Remote: false})
_, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
// Close has no effect
require.ErrorContains(t, str.Close(), "close called for canceled stream")
_, ok, _ = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, _ = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
_, err = (&writerWithTimeout{Writer: str, Timeout: time.Second}).Write([]byte("foobar"))
require.Error(t, err)
// TODO(#4808):error code and remote flag are unchanged
@ -655,8 +655,8 @@ func TestSendStreamCancellationAfterClose(t *testing.T) {
mockSender.EXPECT().onHasStreamControlFrame(streamID, str)
str.CancelWrite(1337)
_, ok, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
cf, ok, hasMore := str.getControlFrame(time.Now())
@ -687,11 +687,11 @@ func testSendStreamCancellationStreamRetransmission(t *testing.T, remote bool) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount).Times(2)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3)).Times(2)
f1, ok, hasMore := str.popStreamFrame(3+expectedFrameHeaderLen(streamID, 0), protocol.Version1)
require.True(t, ok)
f1, hasMore := str.popStreamFrame(3+expectedFrameHeaderLen(streamID, 0), protocol.Version1)
require.NotNil(t, f1.Frame)
require.True(t, hasMore)
f2, ok, hasMore := str.popStreamFrame(3+expectedFrameHeaderLen(streamID, 3), protocol.Version1)
require.True(t, ok)
f2, hasMore := str.popStreamFrame(3+expectedFrameHeaderLen(streamID, 3), protocol.Version1)
require.NotNil(t, f2.Frame)
require.False(t, hasMore)
mockSender.EXPECT().onHasStreamControlFrame(streamID, str)
@ -708,8 +708,8 @@ func testSendStreamCancellationStreamRetransmission(t *testing.T, remote bool) {
// it doesn't matter if the STREAM frames are acked or lost
f1.Handler.OnAcked(f1.Frame)
f2.Handler.OnLost(f2.Frame)
_, ok, hasMore = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, hasMore := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
require.False(t, hasMore)
// if CancelWrite was called, the stream is completed as soon as the RESET_STREAM frame is acked
if !remote {
@ -768,8 +768,8 @@ func TestSendStreamStopSending(t *testing.T) {
require.NoError(t, err)
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(gomock.Any())
_, ok, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
frame, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.NotNil(t, frame.Frame)
require.True(t, mockCtrl.Satisfied())
errChan := make(chan error, 1)
@ -796,8 +796,8 @@ func TestSendStreamStopSending(t *testing.T) {
// calls to Write should return an error
_, err = (&writerWithTimeout{Writer: str, Timeout: time.Second}).Write([]byte("foobar"))
require.ErrorIs(t, err, &StreamError{StreamID: streamID, ErrorCode: 1337, Remote: true})
_, ok, _ = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, _ = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
// calls to CancelWrite have no effect
str.CancelWrite(1234)
@ -809,8 +809,8 @@ func TestSendStreamStopSending(t *testing.T) {
// Close has no effect
require.ErrorContains(t, str.Close(), "close called for canceled stream")
_, ok, _ = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.False(t, ok)
frame, _ = str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.Nil(t, frame.Frame)
_, err = (&writerWithTimeout{Writer: str, Timeout: time.Second}).Write([]byte("foobar"))
require.Error(t, err)
// TODO(#4808):error code and remote flag are unchanged
@ -882,9 +882,11 @@ func TestSendStreamRetransmissions(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3))
f1, ok, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true}, f1.Frame)
f1, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true},
f1.Frame,
)
require.True(t, mockCtrl.Satisfied())
// write some more data
@ -900,8 +902,7 @@ func TestSendStreamRetransmissions(t *testing.T) {
require.True(t, mockCtrl.Satisfied())
// when popping a new frame, we first get the retransmission...
f2, ok, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
f2, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true}, f2.Frame)
require.True(t, hasMoreData)
require.True(t, mockCtrl.Satisfied())
@ -909,8 +910,7 @@ func TestSendStreamRetransmissions(t *testing.T) {
// ... then we get the new data
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(3))
f3, ok, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
f3, hasMoreData := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Offset: 3, Fin: true, Data: []byte("bar"), DataLenPresent: true}, f3.Frame)
require.False(t, hasMoreData)
require.True(t, mockCtrl.Satisfied())
@ -935,31 +935,35 @@ func TestSendStreamRetransmissionFraming(t *testing.T) {
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(6))
f, ok, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
f, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.NotNil(t, f.Frame)
// lose the frame
mockSender.EXPECT().onHasStreamData(streamID, str)
f.Handler.OnLost(f.Frame)
// retransmission doesn't fit
_, ok, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0), protocol.Version1)
require.False(t, ok)
f, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0), protocol.Version1)
require.Nil(t, f.Frame)
require.True(t, hasMore)
// split the retransmission
r1, ok, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0)+3, protocol.Version1)
require.True(t, ok)
r1, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 0)+3, protocol.Version1)
require.True(t, hasMore)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true},
r1.Frame,
)
r2, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 3)+3, protocol.Version1)
require.True(t, hasMore)
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Data: []byte("foo"), DataLenPresent: true}, r1.Frame)
r2, ok, hasMore := str.popStreamFrame(expectedFrameHeaderLen(streamID, 3)+3, protocol.Version1)
require.True(t, ok)
// When popping a retransmission, we always claim that there's more data to send.
// We accept that this might be incorrect.
require.True(t, hasMore)
require.EqualExportedValues(t, &wire.StreamFrame{StreamID: streamID, Offset: 3, Data: []byte("bar"), DataLenPresent: true}, r2.Frame)
_, ok, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 3)+3, protocol.Version1)
require.False(t, ok)
require.EqualExportedValues(t,
&wire.StreamFrame{StreamID: streamID, Offset: 3, Data: []byte("bar"), DataLenPresent: true},
r2.Frame,
)
_, hasMore = str.popStreamFrame(expectedFrameHeaderLen(streamID, 3)+3, protocol.Version1)
require.False(t, hasMore)
}
@ -1000,8 +1004,8 @@ func TestSendStreamRetransmitDataUntilAcknowledged(t *testing.T) {
if completed {
break
}
f, ok, _ := str.popStreamFrame(protocol.ByteCount(mrand.Intn(300)+100), protocol.Version1)
if !ok {
f, _ := str.popStreamFrame(protocol.ByteCount(mrand.Intn(300)+100), protocol.Version1)
if f.Frame == nil {
continue
}
sf := f.Frame

View file

@ -57,7 +57,7 @@ type streamI interface {
// for sending
hasData() bool
handleStopSendingFrame(*wire.StopSendingFrame)
popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool, bool)
popStreamFrame(maxBytes protocol.ByteCount, v protocol.Version) (ackhandler.StreamFrame, bool)
updateSendWindow(protocol.ByteCount)
}

View file

@ -69,8 +69,8 @@ func TestStreamCompletion(t *testing.T) {
require.NoError(t, str.Close())
mockFC.EXPECT().SendWindowSize().Return(protocol.MaxByteCount)
mockFC.EXPECT().AddBytesSent(protocol.ByteCount(6))
f, ok, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.True(t, ok)
f, _ := str.popStreamFrame(protocol.MaxByteCount, protocol.Version1)
require.NotNil(t, f.Frame)
require.True(t, f.Frame.Fin)
f.Handler.OnAcked(f.Frame)
require.True(t, mockCtrl.Satisfied())