From e8de94485c375f07f1cc006c69a9ce40a95d9bf4 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 8 Nov 2018 15:26:54 +0700 Subject: [PATCH] move more stream ID logic to the protocol package --- internal/protocol/stream_id.go | 41 ++++++++++++----------- internal/protocol/stream_id_test.go | 52 +++++++++++++---------------- streams_map.go | 33 +++++------------- streams_map_test.go | 8 ++--- 4 files changed, 56 insertions(+), 78 deletions(-) diff --git a/internal/protocol/stream_id.go b/internal/protocol/stream_id.go index 0e0ab7e9..45f1d464 100644 --- a/internal/protocol/stream_id.go +++ b/internal/protocol/stream_id.go @@ -29,32 +29,33 @@ func (s StreamID) Type() StreamType { return StreamTypeBidi } -// MaxBidiStreamID is the highest stream ID that the peer is allowed to open, -// when it is allowed to open numStreams bidirectional streams. -func MaxBidiStreamID(numStreams int, pers Perspective) StreamID { +// MaxStreamID is the highest stream ID that a peer is allowed to open, +// when it is allowed to open numStreams. +func MaxStreamID(stype StreamType, numStreams int, pers Perspective) StreamID { if numStreams == 0 { return 0 } var first StreamID - if pers == PerspectiveClient { - first = 1 - } else { - first = 0 + switch stype { + case StreamTypeBidi: + switch pers { + case PerspectiveClient: + first = 0 + case PerspectiveServer: + first = 1 + } + case StreamTypeUni: + switch pers { + case PerspectiveClient: + first = 2 + case PerspectiveServer: + first = 3 + } } return first + 4*StreamID(numStreams-1) } -// MaxUniStreamID is the highest stream ID that the peer is allowed to open, -// when it is allowed to open numStreams unidirectional streams. -func MaxUniStreamID(numStreams int, pers Perspective) StreamID { - if numStreams == 0 { - return 0 - } - var first StreamID - if pers == PerspectiveClient { - first = 3 - } else { - first = 2 - } - return first + 4*StreamID(numStreams-1) +// FirstStream returns the first valid stream ID +func FirstStream(stype StreamType, pers Perspective) StreamID { + return MaxStreamID(stype, 1, pers) } diff --git a/internal/protocol/stream_id_test.go b/internal/protocol/stream_id_test.go index e19823d3..669343c8 100644 --- a/internal/protocol/stream_id_test.go +++ b/internal/protocol/stream_id_test.go @@ -20,39 +20,33 @@ var _ = Describe("Stream ID", func() { Expect(StreamID(7).Type()).To(Equal(StreamTypeUni)) }) + It("tells the first stream ID", func() { + Expect(FirstStream(StreamTypeBidi, PerspectiveClient)).To(Equal(StreamID(0))) + Expect(FirstStream(StreamTypeBidi, PerspectiveServer)).To(Equal(StreamID(1))) + Expect(FirstStream(StreamTypeUni, PerspectiveClient)).To(Equal(StreamID(2))) + Expect(FirstStream(StreamTypeUni, PerspectiveServer)).To(Equal(StreamID(3))) + }) + Context("maximum stream IDs", func() { - Context("bidirectional streams", func() { - It("doesn't allow any", func() { - Expect(MaxBidiStreamID(0, PerspectiveClient)).To(Equal(StreamID(0))) - Expect(MaxBidiStreamID(0, PerspectiveServer)).To(Equal(StreamID(0))) - }) - - It("allows one", func() { - Expect(MaxBidiStreamID(1, PerspectiveClient)).To(Equal(StreamID(1))) - Expect(MaxBidiStreamID(1, PerspectiveServer)).To(Equal(StreamID(0))) - }) - - It("allows many", func() { - Expect(MaxBidiStreamID(100, PerspectiveClient)).To(Equal(StreamID(397))) - Expect(MaxBidiStreamID(100, PerspectiveServer)).To(Equal(StreamID(396))) - }) + It("doesn't allow any", func() { + Expect(MaxStreamID(StreamTypeBidi, 0, PerspectiveClient)).To(Equal(StreamID(0))) + Expect(MaxStreamID(StreamTypeBidi, 0, PerspectiveServer)).To(Equal(StreamID(0))) + Expect(MaxStreamID(StreamTypeUni, 0, PerspectiveClient)).To(Equal(StreamID(0))) + Expect(MaxStreamID(StreamTypeUni, 0, PerspectiveServer)).To(Equal(StreamID(0))) }) - Context("unidirectional streams", func() { - It("doesn't allow any", func() { - Expect(MaxUniStreamID(0, PerspectiveClient)).To(Equal(StreamID(0))) - Expect(MaxUniStreamID(0, PerspectiveServer)).To(Equal(StreamID(0))) - }) + It("allows one", func() { + Expect(MaxStreamID(StreamTypeBidi, 1, PerspectiveClient)).To(Equal(StreamID(0))) + Expect(MaxStreamID(StreamTypeBidi, 1, PerspectiveServer)).To(Equal(StreamID(1))) + Expect(MaxStreamID(StreamTypeUni, 1, PerspectiveClient)).To(Equal(StreamID(2))) + Expect(MaxStreamID(StreamTypeUni, 1, PerspectiveServer)).To(Equal(StreamID(3))) + }) - It("allows one", func() { - Expect(MaxUniStreamID(1, PerspectiveClient)).To(Equal(StreamID(3))) - Expect(MaxUniStreamID(1, PerspectiveServer)).To(Equal(StreamID(2))) - }) - - It("allows many", func() { - Expect(MaxUniStreamID(100, PerspectiveClient)).To(Equal(StreamID(399))) - Expect(MaxUniStreamID(100, PerspectiveServer)).To(Equal(StreamID(398))) - }) + It("allows many", func() { + Expect(MaxStreamID(StreamTypeBidi, 100, PerspectiveClient)).To(Equal(StreamID(396))) + Expect(MaxStreamID(StreamTypeBidi, 100, PerspectiveServer)).To(Equal(StreamID(397))) + Expect(MaxStreamID(StreamTypeUni, 100, PerspectiveClient)).To(Equal(StreamID(398))) + Expect(MaxStreamID(StreamTypeUni, 100, PerspectiveServer)).To(Equal(StreamID(399))) }) }) }) diff --git a/streams_map.go b/streams_map.go index 101d4fa6..f909c5f0 100644 --- a/streams_map.go +++ b/streams_map.go @@ -36,18 +36,6 @@ func newStreamsMap( newFlowController: newFlowController, sender: sender, } - var firstOutgoingBidiStream, firstOutgoingUniStream, firstIncomingBidiStream, firstIncomingUniStream protocol.StreamID - if perspective == protocol.PerspectiveServer { - firstOutgoingBidiStream = 1 - firstIncomingBidiStream = 0 - firstOutgoingUniStream = 3 - firstIncomingUniStream = 2 - } else { - firstOutgoingBidiStream = 0 - firstIncomingBidiStream = 1 - firstOutgoingUniStream = 2 - firstIncomingUniStream = 3 - } newBidiStream := func(id protocol.StreamID) streamI { return newStream(id, m.sender, m.newFlowController(id), version) } @@ -58,25 +46,25 @@ func newStreamsMap( return newReceiveStream(id, m.sender, m.newFlowController(id), version) } m.outgoingBidiStreams = newOutgoingBidiStreamsMap( - firstOutgoingBidiStream, + protocol.FirstStream(protocol.StreamTypeBidi, perspective), newBidiStream, sender.queueControlFrame, ) m.incomingBidiStreams = newIncomingBidiStreamsMap( - firstIncomingBidiStream, - protocol.MaxBidiStreamID(maxIncomingStreams, perspective), + protocol.FirstStream(protocol.StreamTypeBidi, perspective.Opposite()), + protocol.MaxStreamID(protocol.StreamTypeBidi, maxIncomingStreams, perspective.Opposite()), maxIncomingStreams, sender.queueControlFrame, newBidiStream, ) m.outgoingUniStreams = newOutgoingUniStreamsMap( - firstOutgoingUniStream, + protocol.FirstStream(protocol.StreamTypeUni, perspective), newUniSendStream, sender.queueControlFrame, ) m.incomingUniStreams = newIncomingUniStreamsMap( - firstIncomingUniStream, - protocol.MaxUniStreamID(maxIncomingUniStreams, perspective), + protocol.FirstStream(protocol.StreamTypeUni, perspective.Opposite()), + protocol.MaxStreamID(protocol.StreamTypeUni, maxIncomingUniStreams, perspective.Opposite()), maxIncomingUniStreams, sender.queueControlFrame, newUniReceiveStream, @@ -174,13 +162,8 @@ func (m *streamsMap) HandleMaxStreamIDFrame(f *wire.MaxStreamIDFrame) error { func (m *streamsMap) UpdateLimits(p *handshake.TransportParameters) { // Max{Uni,Bidi}StreamID returns the highest stream ID that the peer is allowed to open. - // Invert the perspective to determine the value that we are allowed to open. - peerPers := protocol.PerspectiveServer - if m.perspective == protocol.PerspectiveServer { - peerPers = protocol.PerspectiveClient - } - m.outgoingBidiStreams.SetMaxStream(protocol.MaxBidiStreamID(int(p.MaxBidiStreams), peerPers)) - m.outgoingUniStreams.SetMaxStream(protocol.MaxUniStreamID(int(p.MaxUniStreams), peerPers)) + m.outgoingBidiStreams.SetMaxStream(protocol.MaxStreamID(protocol.StreamTypeBidi, int(p.MaxBidiStreams), m.perspective)) + m.outgoingUniStreams.SetMaxStream(protocol.MaxStreamID(protocol.StreamTypeUni, int(p.MaxUniStreams), m.perspective)) } func (m *streamsMap) CloseWithError(err error) { diff --git a/streams_map_test.go b/streams_map_test.go index db6dac38..62dc7709 100644 --- a/streams_map_test.go +++ b/streams_map_test.go @@ -330,19 +330,19 @@ var _ = Describe("Streams Map", func() { Context("sending MAX_STREAM_ID frames", func() { It("sends MAX_STREAM_ID frames for bidirectional streams", func() { - _, err := m.GetOrOpenReceiveStream(ids.firstIncomingBidiStream + 4*10) + _, err := m.GetOrOpenReceiveStream(ids.firstIncomingBidiStream) Expect(err).ToNot(HaveOccurred()) mockSender.EXPECT().queueControlFrame(&wire.MaxStreamIDFrame{ - StreamID: protocol.MaxBidiStreamID(maxBidiStreams, perspective) + 4, + StreamID: ids.firstIncomingBidiStream + 4*maxBidiStreams, }) Expect(m.DeleteStream(ids.firstIncomingBidiStream)).To(Succeed()) }) It("sends MAX_STREAM_ID frames for unidirectional streams", func() { - _, err := m.GetOrOpenReceiveStream(ids.firstIncomingUniStream + 4*10) + _, err := m.GetOrOpenReceiveStream(ids.firstIncomingUniStream) Expect(err).ToNot(HaveOccurred()) mockSender.EXPECT().queueControlFrame(&wire.MaxStreamIDFrame{ - StreamID: protocol.MaxUniStreamID(maxUniStreams, perspective) + 4, + StreamID: ids.firstIncomingUniStream + 4*maxUniStreams, }) Expect(m.DeleteStream(ids.firstIncomingUniStream)).To(Succeed()) })