Skip to content

Commit a0ffa75

Browse files
limit the number of queued PATH_RESPONSE frames to 256 (#4199)
1 parent 7da12a6 commit a0ffa75

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"
@@ -110,6 +111,55 @@ var _ = Describe("Framer", func() {
110111
})
111112
})
112113

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

0 commit comments

Comments
 (0)