commit aa98d2ff4911c71ef71117d55b1213881b13b6cb Author: Dave Rolek dmr-x@riseup.net Date: Wed May 30 20:38:52 2018 +0000

Fix possible packing of CircuitCells with circ_id 0

Also add regression tests, since there were no tests for this behavior. --- stem/client/cell.py | 4 ++-- test/unit/client/cell.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/stem/client/cell.py b/stem/client/cell.py index 9f7e8447..809566a4 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -196,8 +196,8 @@ class Cell(object): :raise: **ValueError** if cell type invalid or payload makes cell too large """

- if isinstance(cls, CircuitCell) and circ_id is None: - raise ValueError('%s cells require a circ_id' % cls.NAME) + if issubclass(cls, CircuitCell) and not circ_id: + raise ValueError('%s cells require a non-zero circ_id' % cls.NAME)

link_protocol = LinkProtocol.for_version(link_protocol)

diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index b6734fe7..f7ab0885 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -14,6 +14,7 @@ from test.unit.client import test_data from stem.client.cell import ( FIXED_PAYLOAD_LEN, Cell, + CircuitCell, PaddingCell, RelayCell, DestroyCell, @@ -122,6 +123,29 @@ class TestCell(unittest.TestCase): expected_message = 'Cell of type OVERSIZED is too large (%i bytes), must not be more than %i. Check payload size (was %i bytes)' % (FIXED_PAYLOAD_LEN + 4, FIXED_PAYLOAD_LEN + 3, FIXED_PAYLOAD_LEN + 1) self.assertRaisesRegexp(ValueError, re.escape(expected_message), instance.pack, 2)

+ def test_circuit_cell_requires_circ_id(self): + class SomeCircuitCell(CircuitCell): + NAME = 'SOME_CIRCUIT_CELL' + VALUE = 127 # currently nonsense, but potentially will be allocated in the distant future + IS_FIXED_SIZE = True + + def __init__(self): + pass # don't take in a circ_id, unlike super() + + def pack(self, link_protocol): + return SomeCircuitCell._pack(link_protocol, b'') + + def pack_with_circ_id(self, link_protocol, circ_id): + return SomeCircuitCell._pack(link_protocol, b'', circ_id) + + instance = SomeCircuitCell() + + expected_message_regex = '^%s$' % re.escape('SOME_CIRCUIT_CELL cells require a non-zero circ_id') + + self.assertRaisesRegexp(ValueError, expected_message_regex, instance.pack, 2) + self.assertRaisesRegexp(ValueError, expected_message_regex, instance.pack_with_circ_id, 2, None) + self.assertRaisesRegexp(ValueError, expected_message_regex, instance.pack_with_circ_id, 2, 0) + def test_unpack_for_new_link(self): expected_certs = ( (CertType.LINK, 1, b'0\x82\x02F0\x82\x01\xaf'),