June 2018 - tor-commits - lists.torproject.org

[stem/master] VPADDING cells can now be constructed correctly with size 0
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit e70dcbda95d4e4cc5c87f1b71d2980d4c031330d Author: Dave Rolek <dmr-x(a)riseup.net> Date: Sun Jun 3 21:16:36 2018 +0000 VPADDING cells can now be constructed correctly with size 0 Unclear if the previous behavior was intentional, but the spec says nothing about requiring size > 0, so making this easier. It was already possible by specifying payload directly, so specifying size should have feature parity. --- stem/client/cell.py | 2 +- test/unit/client/cell.py | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index ce9392b2..3c923b35 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -588,7 +588,7 @@ class VPaddingCell(Cell): def __init__(self, size = None, payload = None): if payload is None: - payload = os.urandom(size) if size else os.urandom(random.randint(128, 1024)) + payload = os.urandom(size) if size is not None else os.urandom(random.randint(128, 1024)) elif size is not None and size != len(payload): raise ValueError('VPaddingCell constructor specified both a size of %i bytes and payload of %i bytes' % (size, len(payload))) diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 76fe17ea..93452195 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -60,8 +60,10 @@ NETINFO_CELLS = { b'\x00\x00\x08ZZ\xb6\x90\x04\x04\x7f\x00\x00\x01\x01\x04\x04aq\x0f\x02' + ZERO * (FIXED_PAYLOAD_LEN - 17): (datetime.datetime(2018, 1, 14, 1, 46, 56), Address('127.0.0.1'), [Address('97.113.15.2')]), } +VPADDING_CELL_EMPTY_PACKED = b'\x00\x00\x80\x00\x00' + VPADDING_CELLS = { - b'\x00\x00\x80\x00\x00': b'', + VPADDING_CELL_EMPTY_PACKED: b'', b'\x00\x00\x80\x00\x01\x08': b'\x08', b'\x00\x00\x80\x00\x02\x08\x11': b'\x08\x11', b'\x00\x00\x80\x01\xfd' + RANDOM_PAYLOAD: RANDOM_PAYLOAD, @@ -213,6 +215,10 @@ class TestCell(unittest.TestCase): self.assertEqual(cell_bytes, VPaddingCell(payload = payload).pack(2)) self.assertEqual(payload, Cell.pop(cell_bytes, 2)[0].payload) + empty_constructed_cell = VPaddingCell(size = 0) + self.assertEqual(VPADDING_CELL_EMPTY_PACKED, empty_constructed_cell.pack(2)) + self.assertEqual(b'', empty_constructed_cell.payload) + self.assertRaisesRegexp(ValueError, 'VPaddingCell constructor specified both a size of 5 bytes and payload of 1 bytes', VPaddingCell, 5, '\x02') def test_certs_cell(self):

1 0

[stem/master] Explicitly test negative VPADDING sizes
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 9040cf46b24946d9cc3a96ab25e5a0af506cdad6 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 16 11:03:08 2018 -0700 Explicitly test negative VPADDING sizes Nitpickiest of nitpicks... * Explicitly checking for negative VPADDING sizes so we can an error message that includes the argument. * Testing for negative sizes in our unit tests. * Flatten our validation checks a bit. --- stem/client/cell.py | 13 ++++++------- test/unit/client/cell.py | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index d5e71e8a..61503c40 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -586,15 +586,14 @@ class VPaddingCell(Cell): IS_FIXED_SIZE = False def __init__(self, size = None, payload = None): - if payload is None: - if size is not None: - payload = os.urandom(size) # enforces size >= 0 - else: - raise ValueError('VPaddingCell constructor must specify payload or size') - elif size is not None and size != len(payload): + if size is None and payload is None: + raise ValueError('VPaddingCell constructor must specify payload or size') + elif size is not None and size < 0: + raise ValueError('VPaddingCell size (%s) cannot be negative' % size) + elif size is not None and payload is not None and size != len(payload): raise ValueError('VPaddingCell constructor specified both a size of %i bytes and payload of %i bytes' % (size, len(payload))) - self.payload = payload + self.payload = payload if payload is not None else os.urandom(size) def pack(self, link_protocol): return VPaddingCell._pack(link_protocol, self.payload) diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 785d48d2..2123cd6f 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -221,7 +221,7 @@ class TestCell(unittest.TestCase): self.assertEqual(b'', empty_constructed_cell.payload) self.assertRaisesRegexp(ValueError, 'VPaddingCell constructor specified both a size of 5 bytes and payload of 1 bytes', VPaddingCell, 5, '\x02') - + self.assertRaisesRegexp(ValueError, re.escape('VPaddingCell size (-15) cannot be negative'), VPaddingCell, -15) self.assertRaisesRegexp(ValueError, '^%s$' % re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell) def test_certs_cell(self):

1 0

[stem/master] Enforce VPaddingCell constructor specification of payload or size
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit c865744d4dded17ad46e637b8e116c41f251fbba Author: Dave Rolek <dmr-x(a)riseup.net> Date: Wed May 30 20:45:15 2018 +0000 Enforce VPaddingCell constructor specification of payload or size The behavior of a default range for VPADDING cells is not specified in the spec, and shouldn't be facilitated by stem.client. This change also removes a potentially network-visible use of insecure random generation. For stem.client randomness, we use secure generation via os.urandom() (Finally, this removes import of the random module, since it is no longer used.) --- stem/client/cell.py | 6 ++++-- test/unit/client/cell.py | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index 3c923b35..d5e71e8a 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -40,7 +40,6 @@ Messages communicated over a Tor relay's ORPort. import datetime import inspect import os -import random import sys import stem.util @@ -588,7 +587,10 @@ class VPaddingCell(Cell): def __init__(self, size = None, payload = None): if payload is None: - payload = os.urandom(size) if size is not None else os.urandom(random.randint(128, 1024)) + if size is not None: + payload = os.urandom(size) # enforces size >= 0 + else: + raise ValueError('VPaddingCell constructor must specify payload or size') elif size is not None and size != len(payload): raise ValueError('VPaddingCell constructor specified both a size of %i bytes and payload of %i bytes' % (size, len(payload))) diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 93452195..785d48d2 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -5,6 +5,7 @@ Unit tests for the stem.client.cell. import datetime import hashlib import os +import re import unittest from stem.client.datatype import ZERO, CertType, CloseReason, Address, Certificate @@ -221,6 +222,8 @@ class TestCell(unittest.TestCase): self.assertRaisesRegexp(ValueError, 'VPaddingCell constructor specified both a size of 5 bytes and payload of 1 bytes', VPaddingCell, 5, '\x02') + self.assertRaisesRegexp(ValueError, '^%s$' % re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell) + def test_certs_cell(self): for cell_bytes, certs in CERTS_CELLS.items(): self.assertEqual(cell_bytes, CertsCell(certs).pack(2))

1 0

[stem/master] Localize protocol attributes into a LinkProtocol class
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 7f7f1714f54326c6f75f5abf42ec8fa059003f11 Merge: 7b3df73f 526dc8b2 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 16 16:44:32 2018 -0700 Localize protocol attributes into a LinkProtocol class Several constants vary depending on our link protocol version. Rather than encoding a bunch of 'x if link_protocol > y else z' conditionals centralizing this in a LinkProtocol class. LinkPrococol.for_version() normalizes an integer *or* LinkProtocol into a LinkProtocol. Pattern we should follow is: * Public methods should always accept an integer. * Any time we use or store a link protocol version it's normalized to a LinkPrococol class. stem/client/__init__.py | 16 +++++----------- stem/client/cell.py | 20 +++++++++++--------- stem/client/datatype.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 20 deletions(-)

1 0

[stem/master] Support empty VPADDING cells
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 7b3df73f3aba7287241d6da3636da53a815c84e2 Merge: 4306013a 9040cf46 Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 16 11:06:11 2018 -0700 Support empty VPADDING cells Good catch from Dave that we should allow VPADDING cells to be created with zero length. Also better constructor validation and behavior. stem/client/cell.py | 11 ++++++----- test/unit/client/cell.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 6 deletions(-)

1 0

[stem/master] Refactor circ_id Size determination into a @staticmethod
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 53d7e471f77c44010161312de3f00f02ea301a28 Author: Dave Rolek <dmr-x(a)riseup.net> Date: Sun May 27 19:47:03 2018 +0000 Refactor circ_id Size determination into a @staticmethod --- stem/client/cell.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index 61503c40..e2aba8b6 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -113,6 +113,22 @@ class Cell(object): raise ValueError("'%s' isn't a valid cell value" % value) + @staticmethod + def _get_circ_id_size(link_protocol): + """ + Gets the proper Size for the link_protocol. + + :param int link_protocol: link protocol version + + :returns: :class:`~stem.client.datatype.Size` + """ + + # per tor-spec section 3 + # CIRCID_LEN := + # 2 for link protocol versions 1, 2, and 3 + # 4 for link protocol versions 4+ + return Size.LONG if link_protocol >= 4 else Size.SHORT + def pack(self, link_protocol): raise NotImplementedError('Unpacking not yet implemented for %s cells' % type(self).NAME) @@ -150,7 +166,7 @@ class Cell(object): * NotImplementedError if unable to unpack this cell type """ - circ_id, content = Size.SHORT.pop(content) if link_protocol < 4 else Size.LONG.pop(content) + circ_id, content = Cell._get_circ_id_size(link_protocol).pop(content) command, content = Size.CHAR.pop(content) cls = Cell.by_value(command) @@ -191,7 +207,7 @@ class Cell(object): raise ValueError('%s cells require a circ_id' % cls.NAME) cell = bytearray() - cell += Size.LONG.pack(circ_id) if link_protocol > 3 else Size.SHORT.pack(circ_id) + cell += Cell._get_circ_id_size(link_protocol).pack(circ_id) cell += Size.CHAR.pack(cls.VALUE) cell += b'' if cls.IS_FIXED_SIZE else Size.SHORT.pack(len(payload)) cell += payload

1 0

[stem/master] Localize protocol attributes into a LinkProtocol class
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 526dc8b27f391d7c24136a2bdc461e514c7da07c Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 16 16:36:04 2018 -0700 Localize protocol attributes into a LinkProtocol class Several constants vary depending on our link protocol version. Rather than encoding a bunch of 'x if link_protocol > y else z' conditionals centralizing this in a LinkProtocol class. LinkPrococol.for_version() normalizes an integer *or* LinkProtocol into a LinkProtocol. Pattern we should follow is: * Public methods should always accept an integer. * Any time we use or store a link protocol version it's normalized to a LinkPrococol class. --- stem/client/__init__.py | 16 +++++----------- stem/client/cell.py | 36 +++++++++++------------------------- stem/client/datatype.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/stem/client/__init__.py b/stem/client/__init__.py index 1d87966f..c0099182 100644 --- a/stem/client/__init__.py +++ b/stem/client/__init__.py @@ -34,7 +34,7 @@ import stem.client.cell import stem.socket import stem.util.connection -from stem.client.datatype import ZERO, Address, Size, KDF, split +from stem.client.datatype import ZERO, LinkProtocol, Address, Size, KDF, split __all__ = [ 'cell', @@ -52,7 +52,7 @@ class Relay(object): """ def __init__(self, orport, link_protocol): - self.link_protocol = link_protocol + self.link_protocol = LinkProtocol.for_version(link_protocol) self._orport = orport self._orport_lock = threading.RLock() self._circuits = {} @@ -151,13 +151,7 @@ class Relay(object): """ with self._orport_lock: - # Find an unused circuit id. Since we're initiating the circuit we pick any - # value from a range that's determined by our link protocol. - - circ_id = 0x80000000 if self.link_protocol > 3 else 0x01 - - while circ_id in self._circuits: - circ_id += 1 + circ_id = max(self._circuits) + 1 if self._circuits else self.link_protocol.first_circ_id create_fast_cell = stem.client.cell.CreateFastCell(circ_id) self._orport.send(create_fast_cell.pack(self.link_protocol)) @@ -239,7 +233,7 @@ class Circuit(object): # doesn't include the initial circuit id and cell type fields. # Circuit ids vary in length depending on the protocol version. - header_size = 5 if self.relay.link_protocol > 3 else 3 + header_size = self.relay.link_protocol.circ_id_size.size + 1 try: cell = stem.client.cell.RelayCell(self.id, command, data, 0, stream_id) @@ -263,7 +257,7 @@ class Circuit(object): raise stem.ProtocolError('Circuit response should be a series of RELAY cells, but received an unexpected size for a response: %i' % len(reply)) while reply: - circ_id, reply = Size.SHORT.pop(reply) if self.relay.link_protocol < 4 else Size.LONG.pop(reply) + circ_id, reply = self.relay.link_protocol.circ_id_size.pop(reply) command, reply = Size.CHAR.pop(reply) payload, reply = split(reply, stem.client.cell.FIXED_PAYLOAD_LEN) diff --git a/stem/client/cell.py b/stem/client/cell.py index e2aba8b6..7e3e78ef 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -45,7 +45,7 @@ import sys import stem.util from stem import UNDEFINED -from stem.client.datatype import HASH_LEN, ZERO, Address, Certificate, CloseReason, RelayCommand, Size, split +from stem.client.datatype import HASH_LEN, ZERO, LinkProtocol, Address, Certificate, CloseReason, RelayCommand, Size, split from stem.util import _hash_attr, datetime_to_unix, str_tools FIXED_PAYLOAD_LEN = 509 @@ -113,22 +113,6 @@ class Cell(object): raise ValueError("'%s' isn't a valid cell value" % value) - @staticmethod - def _get_circ_id_size(link_protocol): - """ - Gets the proper Size for the link_protocol. - - :param int link_protocol: link protocol version - - :returns: :class:`~stem.client.datatype.Size` - """ - - # per tor-spec section 3 - # CIRCID_LEN := - # 2 for link protocol versions 1, 2, and 3 - # 4 for link protocol versions 4+ - return Size.LONG if link_protocol >= 4 else Size.SHORT - def pack(self, link_protocol): raise NotImplementedError('Unpacking not yet implemented for %s cells' % type(self).NAME) @@ -166,7 +150,9 @@ class Cell(object): * NotImplementedError if unable to unpack this cell type """ - circ_id, content = Cell._get_circ_id_size(link_protocol).pop(content) + link_protocol = LinkProtocol.for_version(link_protocol) + + circ_id, content = link_protocol.circ_id_size.pop(content) command, content = Size.CHAR.pop(content) cls = Cell.by_value(command) @@ -206,8 +192,10 @@ class Cell(object): if isinstance(cls, CircuitCell) and circ_id is None: raise ValueError('%s cells require a circ_id' % cls.NAME) + link_protocol = LinkProtocol.for_version(link_protocol) + cell = bytearray() - cell += Cell._get_circ_id_size(link_protocol).pack(circ_id) + cell += link_protocol.circ_id_size.pack(circ_id) cell += Size.CHAR.pack(cls.VALUE) cell += b'' if cls.IS_FIXED_SIZE else Size.SHORT.pack(len(payload)) cell += payload @@ -215,12 +203,10 @@ class Cell(object): # pad fixed sized cells to the required length if cls.IS_FIXED_SIZE: - fixed_cell_len = 514 if link_protocol > 3 else 512 - - if len(cell) > fixed_cell_len: - raise ValueError('Payload of %s is too large (%i bytes), must be less than %i' % (cls.NAME, len(cell), fixed_cell_len)) + if len(cell) > link_protocol.fixed_cell_len: + raise ValueError('Payload of %s is too large (%i bytes), must be less than %i' % (cls.NAME, len(cell), link_protocol.fixed_cell_len)) - cell += ZERO * (fixed_cell_len - len(cell)) + cell += ZERO * (link_protocol.fixed_cell_len - len(cell)) return bytes(cell) @@ -230,7 +216,7 @@ class Cell(object): Subclass implementation for unpacking cell content. :param bytes content: payload to decode - :param int link_protocol: link protocol version + :param stem.client.datatype.LinkProtocol link_protocol: link protocol version :param int circ_id: circuit id cell is for :returns: instance of this cell type diff --git a/stem/client/datatype.py b/stem/client/datatype.py index 75fae662..9812ced0 100644 --- a/stem/client/datatype.py +++ b/stem/client/datatype.py @@ -14,6 +14,8 @@ users.** See our :class:`~stem.client.Relay` the API you probably want. split - splits bytes into substrings + LinkProtocol - ORPort protocol version. + Field - Packable and unpackable datatype. |- Size - Field of a static size. |- Address - Relay address. @@ -228,6 +230,34 @@ def split(content, size): return content[:size], content[size:] +class LinkProtocol(collections.namedtuple('LinkProtocol', ['version', 'circ_id_size', 'fixed_cell_len', 'first_circ_id'])): + """ + Constants that vary by our link protocol version. + + :var int version: link protocol version + :var stem.client.datatype.Size circ_id_size: circuit identifier field size + :var int fixed_cell_length: size of cells with a fixed length + :var int first_circ_id: When creating circuits we pick an unused identifier + from a range that's determined by our link protocol. + """ + + @staticmethod + def for_version(version): + if isinstance(version, LinkProtocol): + return version # already a LinkProtocol + elif isinstance(version, int): + circ_id_size = Size.LONG if version > 3 else Size.SHORT + fixed_cell_len = 514 if version > 3 else 512 + first_circ_id = 0x80000000 if version > 3 else 0x01 + + return LinkProtocol(version, circ_id_size, fixed_cell_len, first_circ_id) + else: + raise TypeError('LinkProtocol.for_version() should receiving an int, not %s' % type(version).__name__) + + def __int__(self): + return self.version + + class Field(object): """ Packable and unpackable datatype.

1 0

[stem/master] Slight wording fixes regarding payload/cell size
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 1f4ae95d7acf4171cd6da8abbb858e31cccb3cd9 Author: Dave Rolek <dmr-x(a)riseup.net> Date: Wed May 30 20:04:34 2018 +0000 Slight wording fixes regarding payload/cell size This ValueError also didn't have a test to it, so add one. --- stem/client/cell.py | 4 ++-- test/unit/client/cell.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index a679968e..b90fb7a6 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -186,7 +186,7 @@ class Cell(object): :return: **bytes** with the encoded payload - :raise: **ValueError** if cell type invalid or payload is too large + :raise: **ValueError** if cell type invalid or payload makes cell too large """ if isinstance(cls, CircuitCell) and circ_id is None: @@ -204,7 +204,7 @@ class Cell(object): if cls.IS_FIXED_SIZE: if len(cell) > link_protocol.fixed_cell_len: - raise ValueError('Payload of %s is too large (%i bytes), must be less than %i' % (cls.NAME, len(cell), link_protocol.fixed_cell_len)) + raise ValueError('Cell of type %s is too large (%i bytes), must not be more than %i. Check payload size (was %i bytes)' % (cls.NAME, len(cell), link_protocol.fixed_cell_len, len(payload))) cell += ZERO * (link_protocol.fixed_cell_len - len(cell)) diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 2fe05369..d36500cc 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -114,6 +114,20 @@ class TestCell(unittest.TestCase): # that's probably not worth it; instead we'll use the instance method _unpack() self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Unpacking not yet implemented for UNIMPLEMENTED cells'), instance._unpack, b'dummy', 0, 2) + def test_payload_too_large(self): + class OversizedCell(Cell): + NAME = 'OVERSIZED' + VALUE = 127 # currently nonsense, but potentially will be allocated in the distant future + IS_FIXED_SIZE = True + + def pack(self, link_protocol): + return OversizedCell._pack(link_protocol, ZERO * (FIXED_PAYLOAD_LEN + 1)) + + instance = OversizedCell() + + 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, '^%s$' % re.escape(expected_message), instance.pack, 2) + def test_unpack_for_new_link(self): expected_certs = ( (CertType.LINK, 1, b'0\x82\x02F0\x82\x01\xaf'),

1 0

[stem/master] Use base Cell class for test_unimplemented_cell_methods
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit 7711050619af1a2f8ecf4aa87f774baa5f367b3b Author: Damian Johnson <atagar(a)torproject.org> Date: Sat Jun 16 16:49:21 2018 -0700 Use base Cell class for test_unimplemented_cell_methods No need to subclass. Making a child doesn't hurt, but it also doesn't provide anything over just instantiating our abstract class. Also dropping the '^%s$' matching from our regex. This isn't wrong at all - in fact, it's a bit better. But it's also something applicable to ~80% of the raise assertions we make. Probably a quirk of mine, but kinda like to avoid the extra verbosity. It's kinda questionable if we should be making these assertions at all. The following makes a case against it... https://stackoverflow.com/questions/8672754/how-to-show-the-error-messages-… Maybe that's why the unittest module doesn't provide it. --- test/unit/client/cell.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index d36500cc..3d054757 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -103,16 +103,10 @@ class TestCell(unittest.TestCase): self.assertRaises(ValueError, Cell.by_value, None) def test_unimplemented_cell_methods(self): - class UnimplementedCell(Cell): - NAME = 'UNIMPLEMENTED' + cell_instance = Cell() - instance = UnimplementedCell() - - self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Packing not yet implemented for UNIMPLEMENTED cells'), instance.pack, 2) - - # ideally we'd test unpacking with Cell.pop(), but that would mean monkey-patching for Cell.pop() to see this UnimplementedCell - # that's probably not worth it; instead we'll use the instance method _unpack() - self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Unpacking not yet implemented for UNIMPLEMENTED cells'), instance._unpack, b'dummy', 0, 2) + self.assertRaisesRegexp(NotImplementedError, re.escape('Packing not yet implemented for UNKNOWN cells'), cell_instance.pack, 2) + self.assertRaisesRegexp(NotImplementedError, re.escape('Unpacking not yet implemented for UNKNOWN cells'), cell_instance._unpack, b'dummy', 0, 2) def test_payload_too_large(self): class OversizedCell(Cell): @@ -126,7 +120,7 @@ class TestCell(unittest.TestCase): instance = OversizedCell() 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, '^%s$' % re.escape(expected_message), instance.pack, 2) + self.assertRaisesRegexp(ValueError, re.escape(expected_message), instance.pack, 2) def test_unpack_for_new_link(self): expected_certs = ( @@ -245,7 +239,7 @@ class TestCell(unittest.TestCase): self.assertRaisesRegexp(ValueError, 'VPaddingCell constructor specified both a size of 5 bytes and payload of 1 bytes', VPaddingCell, 5, '\x02') self.assertRaisesRegexp(ValueError, re.escape('VPaddingCell size (-15) cannot be negative'), VPaddingCell, -15) - self.assertRaisesRegexp(ValueError, '^%s$' % re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell) + self.assertRaisesRegexp(ValueError, re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell) def test_certs_cell(self): for cell_bytes, certs in CERTS_CELLS.items():

1 0

[stem/master] Fix typo in NYI Exception text
by atagar＠torproject.org 16 Jun '18

16 Jun '18

commit e779257c7e7c7cbe630d84e2e46341e7f8b4a16d Author: Dave Rolek <dmr-x(a)riseup.net> Date: Sat May 26 22:02:45 2018 +0000 Fix typo in NYI Exception text Also add test for this text and refactor existing test for other NYI text, since eventually that Cell will get implemented. --- stem/client/cell.py | 2 +- test/unit/client/cell.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/stem/client/cell.py b/stem/client/cell.py index 7e3e78ef..a679968e 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -114,7 +114,7 @@ class Cell(object): raise ValueError("'%s' isn't a valid cell value" % value) def pack(self, link_protocol): - raise NotImplementedError('Unpacking not yet implemented for %s cells' % type(self).NAME) + raise NotImplementedError('Packing not yet implemented for %s cells' % type(self).NAME) @staticmethod def unpack(content, link_protocol): diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 2123cd6f..2fe05369 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -102,8 +102,17 @@ class TestCell(unittest.TestCase): self.assertRaises(ValueError, Cell.by_value, 85) self.assertRaises(ValueError, Cell.by_value, None) - def test_unpack_not_implemented(self): - self.assertRaisesRegexp(NotImplementedError, 'Unpacking not yet implemented for AUTHORIZE cells', Cell.pop, b'\x00\x00\x84\x00\x06\x00\x01\x00\x02\x00\x03', 2) + def test_unimplemented_cell_methods(self): + class UnimplementedCell(Cell): + NAME = 'UNIMPLEMENTED' + + instance = UnimplementedCell() + + self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Packing not yet implemented for UNIMPLEMENTED cells'), instance.pack, 2) + + # ideally we'd test unpacking with Cell.pop(), but that would mean monkey-patching for Cell.pop() to see this UnimplementedCell + # that's probably not worth it; instead we'll use the instance method _unpack() + self.assertRaisesRegexp(NotImplementedError, '^%s$' % re.escape('Unpacking not yet implemented for UNIMPLEMENTED cells'), instance._unpack, b'dummy', 0, 2) def test_unpack_for_new_link(self): expected_certs = (

1 0