diff --git a/connection.go b/connection.go index 8c66cb0baf0..f8bcd613c6f 100644 --- a/connection.go +++ b/connection.go @@ -521,6 +521,9 @@ func (s *connection) run() error { runLoop: for { + if s.framer.QueuedTooManyControlFrames() { + s.closeLocal(&qerr.TransportError{ErrorCode: InternalError}) + } // Close immediately if requested select { case closeErr = <-s.closeChan: diff --git a/framer.go b/framer.go index df8c2841ff9..1e6219a46ca 100644 --- a/framer.go +++ b/framer.go @@ -21,9 +21,19 @@ type framer interface { AppendStreamFrames([]ackhandler.StreamFrame, protocol.ByteCount, protocol.Version) ([]ackhandler.StreamFrame, protocol.ByteCount) Handle0RTTRejection() error + + // QueuedTooManyControlFrames says if the control frame queue exceeded its maximum queue length. + // This is a hack. + // It is easier to implement than propagating an error return value in QueueControlFrame. + // The correct solution would be to queue frames with their respective structs. + // See https://github.com/quic-go/quic-go/issues/4271 for the queueing of stream-related control frames. + QueuedTooManyControlFrames() bool } -const maxPathResponses = 256 +const ( + maxPathResponses = 256 + maxControlFrames = 16 << 10 +) type framerI struct { mutex sync.Mutex @@ -33,9 +43,10 @@ type framerI struct { activeStreams map[protocol.StreamID]struct{} streamQueue ringbuffer.RingBuffer[protocol.StreamID] - controlFrameMutex sync.Mutex - controlFrames []wire.Frame - pathResponses []*wire.PathResponseFrame + controlFrameMutex sync.Mutex + controlFrames []wire.Frame + pathResponses []*wire.PathResponseFrame + queuedTooManyControlFrames bool } var _ framer = &framerI{} @@ -73,6 +84,11 @@ func (f *framerI) QueueControlFrame(frame wire.Frame) { f.pathResponses = append(f.pathResponses, pr) return } + // This is a hack. + if len(f.controlFrames) >= maxControlFrames { + f.queuedTooManyControlFrames = true + return + } f.controlFrames = append(f.controlFrames, frame) } @@ -105,6 +121,10 @@ func (f *framerI) AppendControlFrames(frames []ackhandler.Frame, maxLen protocol return frames, length } +func (f *framerI) QueuedTooManyControlFrames() bool { + return f.queuedTooManyControlFrames +} + func (f *framerI) AddActiveStream(id protocol.StreamID) { f.mutex.Lock() if _, ok := f.activeStreams[id]; !ok { diff --git a/framer_test.go b/framer_test.go index bd376a100a9..6d364423c63 100644 --- a/framer_test.go +++ b/framer_test.go @@ -109,6 +109,23 @@ var _ = Describe("Framer", func() { Expect(fs).To(HaveLen(2)) Expect(length).To(Equal(ping.Length(version) + ncid.Length(version))) }) + + It("detects when too many frames are queued", func() { + for i := 0; i < maxControlFrames-1; i++ { + framer.QueueControlFrame(&wire.PingFrame{}) + framer.QueueControlFrame(&wire.PingFrame{}) + Expect(framer.QueuedTooManyControlFrames()).To(BeFalse()) + frames, _ := framer.AppendControlFrames([]ackhandler.Frame{}, 1, protocol.Version1) + Expect(frames).To(HaveLen(1)) + Expect(framer.(*framerI).controlFrames).To(HaveLen(i + 1)) + } + framer.QueueControlFrame(&wire.PingFrame{}) + Expect(framer.QueuedTooManyControlFrames()).To(BeFalse()) + Expect(framer.(*framerI).controlFrames).To(HaveLen(maxControlFrames)) + framer.QueueControlFrame(&wire.PingFrame{}) + Expect(framer.QueuedTooManyControlFrames()).To(BeTrue()) + Expect(framer.(*framerI).controlFrames).To(HaveLen(maxControlFrames)) + }) }) Context("handling PATH_RESPONSE frames", func() {