diff --git a/framer.go b/framer.go index 9409af4c2ee..d5c61bcf73e 100644 --- a/framer.go +++ b/framer.go @@ -23,6 +23,8 @@ type framer interface { Handle0RTTRejection() error } +const maxPathResponses = 256 + type framerI struct { mutex sync.Mutex @@ -33,6 +35,7 @@ type framerI struct { controlFrameMutex sync.Mutex controlFrames []wire.Frame + pathResponses []*wire.PathResponseFrame } var _ framer = &framerI{} @@ -52,20 +55,43 @@ func (f *framerI) HasData() bool { return true } f.controlFrameMutex.Lock() - hasData = len(f.controlFrames) > 0 - f.controlFrameMutex.Unlock() - return hasData + defer f.controlFrameMutex.Unlock() + return len(f.controlFrames) > 0 || len(f.pathResponses) > 0 } func (f *framerI) QueueControlFrame(frame wire.Frame) { f.controlFrameMutex.Lock() + defer f.controlFrameMutex.Unlock() + + if pr, ok := frame.(*wire.PathResponseFrame); ok { + // Only queue up to maxPathResponses PATH_RESPONSE frames. + // This limit should be high enough to never be hit in practice, + // unless the peer is doing something malicious. + if len(f.pathResponses) >= maxPathResponses { + return + } + f.pathResponses = append(f.pathResponses, pr) + return + } f.controlFrames = append(f.controlFrames, frame) - f.controlFrameMutex.Unlock() } func (f *framerI) AppendControlFrames(frames []ackhandler.Frame, maxLen protocol.ByteCount, v protocol.VersionNumber) ([]ackhandler.Frame, protocol.ByteCount) { - var length protocol.ByteCount f.controlFrameMutex.Lock() + defer f.controlFrameMutex.Unlock() + + var length protocol.ByteCount + // add a PATH_RESPONSE first, but only pack a single PATH_RESPONSE per packet + if len(f.pathResponses) > 0 { + frame := f.pathResponses[0] + frameLen := frame.Length(v) + if frameLen <= maxLen { + frames = append(frames, ackhandler.Frame{Frame: frame}) + length += frameLen + f.pathResponses = f.pathResponses[1:] + } + } + for len(f.controlFrames) > 0 { frame := f.controlFrames[len(f.controlFrames)-1] frameLen := frame.Length(v) @@ -76,7 +102,6 @@ func (f *framerI) AppendControlFrames(frames []ackhandler.Frame, maxLen protocol length += frameLen f.controlFrames = f.controlFrames[:len(f.controlFrames)-1] } - f.controlFrameMutex.Unlock() return frames, length } diff --git a/framer_test.go b/framer_test.go index add9da713c5..97cbd144bed 100644 --- a/framer_test.go +++ b/framer_test.go @@ -2,7 +2,8 @@ package quic import ( "bytes" - "math/rand" + + "golang.org/x/exp/rand" "github.com/quic-go/quic-go/internal/ackhandler" "github.com/quic-go/quic-go/internal/protocol" @@ -110,6 +111,55 @@ var _ = Describe("Framer", func() { }) }) + Context("handling PATH_RESPONSE frames", func() { + It("packs a single PATH_RESPONSE per packet", func() { + f1 := &wire.PathResponseFrame{Data: [8]byte{1, 2, 3, 4, 5, 6, 7, 8}} + f2 := &wire.PathResponseFrame{Data: [8]byte{2, 3, 4, 5, 6, 7, 8, 9}} + cf1 := &wire.DataBlockedFrame{MaximumData: 1337} + cf2 := &wire.HandshakeDoneFrame{} + framer.QueueControlFrame(f1) + framer.QueueControlFrame(f2) + framer.QueueControlFrame(cf1) + framer.QueueControlFrame(cf2) + // the first packet should contain a single PATH_RESPONSE frame, but all the other control frames + Expect(framer.HasData()).To(BeTrue()) + frames, length := framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1) + Expect(frames).To(HaveLen(3)) + Expect(frames[0].Frame).To(Equal(f1)) + Expect([]wire.Frame{frames[1].Frame, frames[2].Frame}).To(ContainElement(cf1)) + Expect([]wire.Frame{frames[1].Frame, frames[2].Frame}).To(ContainElement(cf2)) + Expect(length).To(Equal(f1.Length(protocol.Version1) + cf1.Length(protocol.Version1) + cf2.Length(protocol.Version1))) + // the second packet should contain the other PATH_RESPONSE frame + Expect(framer.HasData()).To(BeTrue()) + frames, length = framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1) + Expect(frames).To(HaveLen(1)) + Expect(frames[0].Frame).To(Equal(f2)) + Expect(length).To(Equal(f2.Length(protocol.Version1))) + Expect(framer.HasData()).To(BeFalse()) + }) + + It("limits the number of queued PATH_RESPONSE frames", func() { + var pathResponses []*wire.PathResponseFrame + for i := 0; i < 2*maxPathResponses; i++ { + var f wire.PathResponseFrame + rand.Read(f.Data[:]) + pathResponses = append(pathResponses, &f) + framer.QueueControlFrame(&f) + } + for i := 0; i < maxPathResponses; i++ { + Expect(framer.HasData()).To(BeTrue()) + frames, length := framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1) + Expect(frames).To(HaveLen(1)) + Expect(frames[0].Frame).To(Equal(pathResponses[i])) + Expect(length).To(Equal(pathResponses[i].Length(protocol.Version1))) + } + Expect(framer.HasData()).To(BeFalse()) + frames, length := framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1) + Expect(frames).To(BeEmpty()) + Expect(length).To(BeZero()) + }) + }) + Context("popping STREAM frames", func() { It("returns nil when popping an empty framer", func() { Expect(framer.AppendStreamFrames(nil, 1000, protocol.Version1)).To(BeEmpty())