Skip to content

Commit 1d6321a

Browse files
committed
Forbid too many streams being added to priority tree.
1 parent 4374888 commit 1d6321a

File tree

3 files changed

+54
-2
lines changed

3 files changed

+54
-2
lines changed

src/priority/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@
44
"""
55
from .priority import ( # noqa
66
Stream, PriorityTree, DeadlockError, PriorityLoop, DuplicateStreamError,
7-
MissingStreamError
7+
MissingStreamError, TooManyStreamsError
88
)

src/priority/priority.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ class MissingStreamError(KeyError, Exception):
4343
pass
4444

4545

46+
class TooManyStreamsError(Exception):
47+
"""
48+
An attempt was made to insert a dangerous number of streams into the
49+
priority tree at the same time.
50+
51+
.. versionadded:: 1.2.0
52+
"""
53+
pass
54+
55+
4656
class Stream(object):
4757
"""
4858
Priority information for a given stream.
@@ -199,13 +209,33 @@ class PriorityTree(object):
199209
A HTTP/2 Priority Tree.
200210
201211
This tree stores HTTP/2 streams according to their HTTP/2 priorities.
212+
213+
.. versionchanged:: 1.2.0
214+
Added ``maximum_streams`` keyword argument.
215+
216+
:param maximum_streams: The maximum number of streams that may be active in
217+
the priority tree at any one time. If this number is exceeded, the
218+
priority tree will raise a :class:`TooManyStreamsError
219+
<priority.TooManyStreamsError>` and will refuse to insert the stream.
220+
221+
This parameter exists to defend against the possibility of DoS attack
222+
by attempting to overfill the priority tree. If any endpoint is
223+
attempting to manage the priority of this many streams at once it is
224+
probably trying to screw with you, so it is sensible to simply refuse
225+
to play ball at that point.
226+
227+
While we allow the user to configure this, we don't really *expect*
228+
them too, unless they want to be even more conservative than we are by
229+
default.
230+
:type maximum_streams: ``int``
202231
"""
203-
def __init__(self):
232+
def __init__(self, maximum_streams=1000):
204233
# This flat array keeps hold of all the streams that are logically
205234
# dependent on stream 0.
206235
self._root_stream = Stream(stream_id=0, weight=1)
207236
self._root_stream.active = False
208237
self._streams = {0: self._root_stream}
238+
self._maximum_streams = maximum_streams
209239

210240
def _exclusive_insert(self, parent_stream, inserted_stream):
211241
"""
@@ -233,6 +263,13 @@ def insert_stream(self,
233263
if stream_id in self._streams:
234264
raise DuplicateStreamError("Stream %d already in tree" % stream_id)
235265

266+
if (len(self._streams) + 1) > self._maximum_streams:
267+
raise TooManyStreamsError(
268+
"Refusing to insert %d streams into priority tree at once" % (
269+
self._maximum_streams + 1
270+
)
271+
)
272+
236273
stream = Stream(stream_id, weight)
237274

238275
if exclusive:

test/test_priority.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,21 @@ def test_priority_raises_good_errors_for_missing_streams(self):
292292
with pytest.raises(priority.MissingStreamError):
293293
p.remove_stream(3)
294294

295+
@pytest.mark.parametrize('count', range(2, 10000, 100))
296+
def test_priority_refuses_to_allow_too_many_streams_in_tree(self, count):
297+
"""
298+
Attempting to insert more streams than maximum_streams into the tree
299+
fails.
300+
"""
301+
p = priority.PriorityTree(maximum_streams=count)
302+
303+
# This isn't an off-by-one error: stream 0 is in the tree by default.
304+
for x in range(1, count):
305+
p.insert_stream(x)
306+
307+
with pytest.raises(priority.TooManyStreamsError):
308+
p.insert_stream(x + 1)
309+
295310

296311
class TestPriorityTreeOutput(object):
297312
"""

0 commit comments

Comments
 (0)