[tor-commits] [stem/master] Fix possible packing of CircuitCells with circ_id 0

[atagar@xxxxxxxxxxxxxx]

commit aa98d2ff4911c71ef71117d55b1213881b13b6cb
Author: Dave Rolek <dmr-x@xxxxxxxxxx>
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'),



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits