diff --git a/packet_packer.go b/packet_packer.go index ef408015..0d691c50 100644 --- a/packet_packer.go +++ b/packet_packer.go @@ -40,26 +40,29 @@ func newPacketPacker(connectionID protocol.ConnectionID, cryptoSetup *handshake. } } -func (p *packetPacker) PackConnectionClose(frame *frames.ConnectionCloseFrame, leastUnacked protocol.PacketNumber) (*packedPacket, error) { - return p.packPacket(nil, []frames.Frame{frame}, leastUnacked, true) +// PackConnectionClose packs a packet that ONLY contains a ConnectionCloseFrame +func (p *packetPacker) PackConnectionClose(ccf *frames.ConnectionCloseFrame, leastUnacked protocol.PacketNumber) (*packedPacket, error) { + // in case the connection is closed, all queued control frames aren't of any use anymore + // discard them and queue the ConnectionCloseFrame + p.controlFrames = []frames.Frame{ccf} + return p.packPacket(nil, leastUnacked) } +// PackPacket packs a new packet +// the stopWaitingFrame is *guaranteed* to be included in the next packet +// the other controlFrames are sent in the next packet, but might be queued and sent in the next packet if the packet would overflow MaxPacketSize otherwise func (p *packetPacker) PackPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, leastUnacked protocol.PacketNumber) (*packedPacket, error) { - return p.packPacket(stopWaitingFrame, controlFrames, leastUnacked, false) + p.controlFrames = append(p.controlFrames, controlFrames...) + return p.packPacket(stopWaitingFrame, leastUnacked) } -func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, controlFrames []frames.Frame, leastUnacked protocol.PacketNumber, onlySendOneControlFrame bool) (*packedPacket, error) { - if len(controlFrames) > 0 { - p.controlFrames = append(p.controlFrames, controlFrames...) - } - - currentPacketNumber := p.packetNumberGenerator.Peek() - +func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, leastUnacked protocol.PacketNumber) (*packedPacket, error) { // cryptoSetup needs to be locked here, so that the AEADs are not changed between // calling DiversificationNonce() and Seal(). p.cryptoSetup.LockForSealing() defer p.cryptoSetup.UnlockForSealing() + currentPacketNumber := p.packetNumberGenerator.Peek() packetNumberLen := protocol.GetPacketNumberLengthForPublicHeader(currentPacketNumber, leastUnacked) responsePublicHeader := &PublicHeader{ ConnectionID: p.connectionID, @@ -79,9 +82,15 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, con stopWaitingFrame.PacketNumberLen = packetNumberLen } + // we're packing a ConnectionClose, don't add any StreamFrames + var isConnectionClose bool + if len(p.controlFrames) == 1 { + _, isConnectionClose = p.controlFrames[0].(*frames.ConnectionCloseFrame) + } + var payloadFrames []frames.Frame - if onlySendOneControlFrame { - payloadFrames = []frames.Frame{controlFrames[0]} + if isConnectionClose { + payloadFrames = []frames.Frame{p.controlFrames[0]} } else { payloadFrames, err = p.composeNextPacket(stopWaitingFrame, publicHeaderLength) if err != nil { @@ -94,7 +103,7 @@ func (p *packetPacker) packPacket(stopWaitingFrame *frames.StopWaitingFrame, con return nil, nil } // Don't send out packets that only contain a StopWaitingFrame - if !onlySendOneControlFrame && len(payloadFrames) == 1 && stopWaitingFrame != nil { + if len(payloadFrames) == 1 && stopWaitingFrame != nil { return nil, nil } diff --git a/packet_packer_test.go b/packet_packer_test.go index 05b5111b..cd22f6d9 100644 --- a/packet_packer_test.go +++ b/packet_packer_test.go @@ -57,7 +57,7 @@ var _ = Describe("Packet packer", func() { Expect(p.raw).To(ContainSubstring(string(b.Bytes()))) }) - It("packs a ConnectionCloseFrame", func() { + It("packs a ConnectionClose", func() { ccf := frames.ConnectionCloseFrame{ ErrorCode: 0x1337, ReasonPhrase: "foobar", @@ -68,22 +68,27 @@ var _ = Describe("Packet packer", func() { Expect(p.frames[0]).To(Equal(&ccf)) }) - It("ignores all other frames when called with onlySendOneControlFrame=true", func() { + It("doesn't send any other frames when sending a ConnectionClose", func() { ccf := frames.ConnectionCloseFrame{ ErrorCode: 0x1337, ReasonPhrase: "foobar", } - p, err := packer.packPacket(&frames.StopWaitingFrame{LeastUnacked: 13}, []frames.Frame{&ccf, &frames.WindowUpdateFrame{StreamID: 37}}, 0, true) + packer.controlFrames = []frames.Frame{&frames.WindowUpdateFrame{StreamID: 37}} + streamFramer.AddFrameForRetransmission(&frames.StreamFrame{ + StreamID: 5, + Data: []byte("foobar"), + }) + p, err := packer.PackConnectionClose(&ccf, 0) Expect(err).ToNot(HaveOccurred()) Expect(p.frames).To(HaveLen(1)) Expect(p.frames[0]).To(Equal(&ccf)) }) It("packs only control frames", func() { - p, err := packer.PackPacket(nil, []frames.Frame{&frames.RstStreamFrame{}}, 0) + p, err := packer.PackPacket(nil, []frames.Frame{&frames.RstStreamFrame{}, &frames.WindowUpdateFrame{}}, 0) Expect(p).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) - Expect(p.frames).To(HaveLen(1)) + Expect(p.frames).To(HaveLen(2)) Expect(p.raw).NotTo(BeEmpty()) }) @@ -413,4 +418,13 @@ var _ = Describe("Packet packer", func() { Expect(err).NotTo(HaveOccurred()) Expect(p).ToNot(BeNil()) }) + + It("queues a control frame to be sent in the next packet", func() { + wuf := &frames.WindowUpdateFrame{StreamID: 5} + packer.QueueControlFrameForNextPacket(wuf) + p, err := packer.PackPacket(nil, nil, 0) + Expect(err).NotTo(HaveOccurred()) + Expect(p.frames).To(HaveLen(1)) + Expect(p.frames[0]).To(Equal(wuf)) + }) })