Skip to content

Commit d7aa627

Browse files
limit the number of queued PATH_RESPONSE frames to 256 (#4199)
1 parent e2c360c commit d7aa627

File tree

2 files changed

+82
-7
lines changed

2 files changed

+82
-7
lines changed

Diff for: framer.go

+31-6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ type framer interface {
2323
Handle0RTTRejection() error
2424
}
2525

26+
const maxPathResponses = 256
27+
2628
type framerI struct {
2729
mutex sync.Mutex
2830

@@ -33,6 +35,7 @@ type framerI struct {
3335

3436
controlFrameMutex sync.Mutex
3537
controlFrames []wire.Frame
38+
pathResponses []*wire.PathResponseFrame
3639
}
3740

3841
var _ framer = &framerI{}
@@ -52,20 +55,43 @@ func (f *framerI) HasData() bool {
5255
return true
5356
}
5457
f.controlFrameMutex.Lock()
55-
hasData = len(f.controlFrames) > 0
56-
f.controlFrameMutex.Unlock()
57-
return hasData
58+
defer f.controlFrameMutex.Unlock()
59+
return len(f.controlFrames) > 0 || len(f.pathResponses) > 0
5860
}
5961

6062
func (f *framerI) QueueControlFrame(frame wire.Frame) {
6163
f.controlFrameMutex.Lock()
64+
defer f.controlFrameMutex.Unlock()
65+
66+
if pr, ok := frame.(*wire.PathResponseFrame); ok {
67+
// Only queue up to maxPathResponses PATH_RESPONSE frames.
68+
// This limit should be high enough to never be hit in practice,
69+
// unless the peer is doing something malicious.
70+
if len(f.pathResponses) >= maxPathResponses {
71+
return
72+
}
73+
f.pathResponses = append(f.pathResponses, pr)
74+
return
75+
}
6276
f.controlFrames = append(f.controlFrames, frame)
63-
f.controlFrameMutex.Unlock()
6477
}
6578

6679
func (f *framerI) AppendControlFrames(frames []ackhandler.Frame, maxLen protocol.ByteCount, v protocol.VersionNumber) ([]ackhandler.Frame, protocol.ByteCount) {
67-
var length protocol.ByteCount
6880
f.controlFrameMutex.Lock()
81+
defer f.controlFrameMutex.Unlock()
82+
83+
var length protocol.ByteCount
84+
// add a PATH_RESPONSE first, but only pack a single PATH_RESPONSE per packet
85+
if len(f.pathResponses) > 0 {
86+
frame := f.pathResponses[0]
87+
frameLen := frame.Length(v)
88+
if frameLen <= maxLen {
89+
frames = append(frames, ackhandler.Frame{Frame: frame})
90+
length += frameLen
91+
f.pathResponses = f.pathResponses[1:]
92+
}
93+
}
94+
6995
for len(f.controlFrames) > 0 {
7096
frame := f.controlFrames[len(f.controlFrames)-1]
7197
frameLen := frame.Length(v)
@@ -76,7 +102,6 @@ func (f *framerI) AppendControlFrames(frames []ackhandler.Frame, maxLen protocol
76102
length += frameLen
77103
f.controlFrames = f.controlFrames[:len(f.controlFrames)-1]
78104
}
79-
f.controlFrameMutex.Unlock()
80105
return frames, length
81106
}
82107

Diff for: framer_test.go

+51-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ package quic
22

33
import (
44
"bytes"
5-
"math/rand"
5+
6+
"golang.org/x/exp/rand"
67

78
"github.com/quic-go/quic-go/internal/ackhandler"
89
"github.com/quic-go/quic-go/internal/protocol"
@@ -111,6 +112,55 @@ var _ = Describe("Framer", func() {
111112
})
112113
})
113114

115+
Context("handling PATH_RESPONSE frames", func() {
116+
It("packs a single PATH_RESPONSE per packet", func() {
117+
f1 := &wire.PathResponseFrame{Data: [8]byte{1, 2, 3, 4, 5, 6, 7, 8}}
118+
f2 := &wire.PathResponseFrame{Data: [8]byte{2, 3, 4, 5, 6, 7, 8, 9}}
119+
cf1 := &wire.DataBlockedFrame{MaximumData: 1337}
120+
cf2 := &wire.HandshakeDoneFrame{}
121+
framer.QueueControlFrame(f1)
122+
framer.QueueControlFrame(f2)
123+
framer.QueueControlFrame(cf1)
124+
framer.QueueControlFrame(cf2)
125+
// the first packet should contain a single PATH_RESPONSE frame, but all the other control frames
126+
Expect(framer.HasData()).To(BeTrue())
127+
frames, length := framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1)
128+
Expect(frames).To(HaveLen(3))
129+
Expect(frames[0].Frame).To(Equal(f1))
130+
Expect([]wire.Frame{frames[1].Frame, frames[2].Frame}).To(ContainElement(cf1))
131+
Expect([]wire.Frame{frames[1].Frame, frames[2].Frame}).To(ContainElement(cf2))
132+
Expect(length).To(Equal(f1.Length(protocol.Version1) + cf1.Length(protocol.Version1) + cf2.Length(protocol.Version1)))
133+
// the second packet should contain the other PATH_RESPONSE frame
134+
Expect(framer.HasData()).To(BeTrue())
135+
frames, length = framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1)
136+
Expect(frames).To(HaveLen(1))
137+
Expect(frames[0].Frame).To(Equal(f2))
138+
Expect(length).To(Equal(f2.Length(protocol.Version1)))
139+
Expect(framer.HasData()).To(BeFalse())
140+
})
141+
142+
It("limits the number of queued PATH_RESPONSE frames", func() {
143+
var pathResponses []*wire.PathResponseFrame
144+
for i := 0; i < 2*maxPathResponses; i++ {
145+
var f wire.PathResponseFrame
146+
rand.Read(f.Data[:])
147+
pathResponses = append(pathResponses, &f)
148+
framer.QueueControlFrame(&f)
149+
}
150+
for i := 0; i < maxPathResponses; i++ {
151+
Expect(framer.HasData()).To(BeTrue())
152+
frames, length := framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1)
153+
Expect(frames).To(HaveLen(1))
154+
Expect(frames[0].Frame).To(Equal(pathResponses[i]))
155+
Expect(length).To(Equal(pathResponses[i].Length(protocol.Version1)))
156+
}
157+
Expect(framer.HasData()).To(BeFalse())
158+
frames, length := framer.AppendControlFrames(nil, protocol.MaxByteCount, protocol.Version1)
159+
Expect(frames).To(BeEmpty())
160+
Expect(length).To(BeZero())
161+
})
162+
})
163+
114164
Context("popping STREAM frames", func() {
115165
It("returns nil when popping an empty framer", func() {
116166
Expect(framer.AppendStreamFrames(nil, 1000, protocol.Version1)).To(BeEmpty())

0 commit comments

Comments
 (0)