commit 6938036f28ee8e74b409014919b438da7a12fe10
Author: Damian Johnson <atagar(a)torproject.org>
Date: Sun Aug 26 12:30:46 2018 -0700
Drop alternate relay cell classes
Now that our RelayCell implements encryption/decryption we can drop these.
These new methods have *not* been exercised yet. No doubt when rubber meets the
road I'll discover some issues, but they're close enough to the Circuit's prior
send() method that they should be simple to sort out.
---
stem/client/cell.py | 455 +----------------------------------------------
test/unit/client/cell.py | 67 ++-----
test/unit/client/size.py | 1 -
3 files changed, 12 insertions(+), 511 deletions(-)
diff --git a/stem/client/cell.py b/stem/client/cell.py
index d4540203..d7053eaf 100644
--- a/stem/client/cell.py
+++ b/stem/client/cell.py
@@ -108,7 +108,7 @@ class Cell(object):
"""
for _, cls in inspect.getmembers(sys.modules[__name__]):
- if name == getattr(cls, 'NAME', UNDEFINED) and not getattr(cls, 'CANNOT_DIRECTLY_UNPACK', False):
+ if name == getattr(cls, 'NAME', UNDEFINED):
return cls
raise ValueError("'%s' isn't a valid cell type" % name)
@@ -124,7 +124,7 @@ class Cell(object):
"""
for _, cls in inspect.getmembers(sys.modules[__name__]):
- if value == getattr(cls, 'VALUE', UNDEFINED) and not getattr(cls, 'CANNOT_DIRECTLY_UNPACK', False):
+ if value == getattr(cls, 'VALUE', UNDEFINED):
return cls
raise ValueError("'%s' isn't a valid cell value" % value)
@@ -321,199 +321,6 @@ class CreatedCell(CircuitCell):
super(CreatedCell, self).__init__() # TODO: implement
-class BaseRelayCell(CircuitCell):
- """
- Cell whose subclasses are relayed over circuits.
-
- :var bytes payload: raw payload, quite possibly encrypted
- """
-
- NAME = 'INTERNAL_BASE_RELAY' # defined for error/other strings
- IS_FIXED_SIZE = True # all relay cells are fixed-size
-
- # other attributes are deferred to subclasses, since this class cannot be directly unpacked
-
- def __init__(self, circ_id, payload):
- if not payload:
- raise ValueError('Relay cells require a payload')
- if len(payload) != FIXED_PAYLOAD_LEN:
- raise ValueError('Payload should be %i bytes, but was %i' % (FIXED_PAYLOAD_LEN, len(payload)))
-
- super(BaseRelayCell, self).__init__(circ_id, unused = b'')
- self.payload = payload
-
- def pack(self, link_protocol):
- # unlike everywhere else, we actually want to use the subclass type, NOT *this* class
- return type(self)._pack(link_protocol, self.payload, circ_id = self.circ_id)
-
- @classmethod
- def _unpack(cls, content, circ_id, link_protocol):
- # unlike everywhere else, we actually want to use the subclass type, NOT *this* class
- return cls(circ_id, content)
-
- def check_recognized_field(self):
- """
- Checks the 'recognized' field of the cell payload, which indicates whether
- it is **probably** fully decrypted.
-
- :returns: **bool** indicating whether the 'recognized' field indicates
- likely decryption. Per the spec:
- * **False** guarantees the cell *not* to be fully decrypted.
- * **True** does *not* guarantee the cell to be fully decrypted, and it
- must be checked further. See also
- :func:`~stem.client.cell.BaseRelayCell.check_digest`
- """
-
- _, recognized_from_cell, _, _, _, _, _ = AlternateRelayCell._unpack_payload(self.payload)
- return recognized_from_cell == 0
-
- def check_digest(self, digest):
- """
- Calculates the running digest of the cell payload per the spec, returning
- whether the cell's unpacked digest matched, along with the updated digest
- if so.
-
- :param HASH digest: running digest held with the relay
-
- :returns: (digest_matches, digest) tuple of object copies updated as follows:
- * digest_matches: **bool** indicating whether the digest matches
- * digest: updated via digest.update(payload), if the digest matches;
- otherwise a copy of the original
-
- :raises: **ValueError** if payload is the wrong size
- """
-
- command, recognized, stream_id, digest_from_cell, data_len, data, unused = AlternateRelayCell._unpack_payload(self.payload)
-
- # running digest is calculated using a zero'd digest field in the payload
- prepared_payload = AlternateRelayCell._pack_payload(command, recognized, stream_id, 0, data_len, data, unused, pad_remainder = False)
-
- if len(prepared_payload) != FIXED_PAYLOAD_LEN:
- # this should never fail
- # if it did, it indicates a programming error either within stem.client.cell or a consumer
- raise ValueError('Payload should be %i bytes, but was %i' % (FIXED_PAYLOAD_LEN, len(prepared_payload)))
-
- new_digest = digest.copy()
- new_digest.update(prepared_payload)
-
- digest_matches = (AlternateRelayCell._coerce_digest(new_digest) == digest_from_cell)
-
- # only return the new_digest if the digest check passed
- # even if not, return a copy of the original
- # this allows a consumer to always assume the returned digest is a different object
- digest_to_return = new_digest if digest_matches else digest.copy()
-
- return digest_matches, digest_to_return
-
- def interpret_cell(self):
- """
- Interprets the cell payload, returning a new
- :class:`~stem.client.cell.RelayCell` class or subclass according to its
- contents.
-
- This method should only be used on fully decrypted cells, but that
- responsibility is relegated to the caller.
-
- Furthermore, this interpretation may cause an exception for a NYI relay
- command, a malformed cell, or some other reason.
-
- :returns: :class:`~stem.client.cell.RelayCell` class or subclass
- """
-
- # TODO: this mapping is quite hardcoded right now, but probably needs to be
- # completely reworked once the Cell class hierarchy is better fleshed out.
- #
- # (It doesn't really make sense to have anything beyond this hack in the
- # interim.)
- #
- # At that time, it would probably be modeled after Cell.by_value(), albeit
- # specialized for the multiple types of RELAY / RELAY_EARLY cells.
-
- relay_cells_by_value = {
- RawRelayCell.VALUE: RelayCell,
- RelayEarlyCell.VALUE: RelayEarlyCell,
- }
- new_cls = relay_cells_by_value[self.VALUE]
-
- dummy_link_protocol = None
- new_cell = new_cls._unpack(self.payload, self.circ_id, dummy_link_protocol)
-
- return new_cell
-
- def decrypt(self, digest, decryptor, interpret = False):
- """
- Decrypts a cell and checks whether it is fully decrypted,
- returning a new (Cell, fully_decrypted, digest, decryptor) tuple.
- Optionally also interprets the cell (not generally recommended).
-
- The method name is technically a misnomer, as it also checks whether the
- cell has been fully decrypted (after decrypting), updating the digest if so.
- However, these operations are defined per the spec as required for RELAY
- cells, and ...
- (1) it is a natural mental extension to include them here;
- (2) it would be a bit pointless to require method consumers to manually
- do all of that, for pedantry.
-
- :param HASH digest: running digest held with the relay
- :param cryptography.hazmat.primitives.ciphers.CipherContext decryptor:
- running stream cipher decryptor held with the relay
-
- :param bool interpret: (optional, defaults to **False**) Use **True** with
- caution. The spec indicates that a fully decrypted cell should be
- accounted for in digest and decryptor, independent of cell validity. Using
- **True**, while convenient, may cause an exception for a NYI relay
- command, a malformed cell, or some other reason. This option should only
- be used when the consumer will consider the circuit to have a fatal error
- in such cases, and catches/handles the exception accordingly (e.g. sending
- a DestroyCell).
-
- :returns: (:class:`~stem.client.cell.Cell`, bool, HASH, CipherContext) tuple
- of object copies updated as follows:
- * Cell: either :class:`~stem.client.cell.RawRelayCell` with a decrypted
- payload or :class:`~stem.client.cell.RelayCell` class or subclass, if
- **interpret** is **True** and the cell was fully decrypted
- * fully_decrypted: **bool** indicating whether the cell is fully
- decrypted
- * digest: updated via digest.update(payload), if the cell was fully
- decrypted; otherwise a copy of the original
- * decryptor: updated via decryptor.update(payload)
- """
-
- new_decryptor = copy.copy(decryptor)
-
- # actually decrypt
- decrypted_payload = new_decryptor.update(self.payload)
- new_cell = self.__class__(self.circ_id, decrypted_payload)
-
- # do post-decryption checks to ascertain whether cell is fully decrypted
- if new_cell.check_recognized_field():
- digest_matches, new_digest = new_cell.check_digest(digest)
- fully_decrypted = digest_matches
- else:
- new_digest = None
- fully_decrypted = False
-
- # only return the new_digest if the digest check meant that the cell has been fully decrypted
- #
- # furthermore, even if the digest was not updated, return a copy
- # this allows a consumer to always assume the returned digest is a different object
- digest_to_return = new_digest if fully_decrypted else digest.copy()
-
- if interpret and fully_decrypted:
- # this might raise an exception; oh well, we did warn about that
- new_cell = new_cell.interpret_cell()
-
- return new_cell, fully_decrypted, digest_to_return, new_decryptor
-
- def __hash__(self):
- return stem.util._hash_attr(self, 'circ_id', 'payload', cache = True)
-
-
-class RawRelayCell(BaseRelayCell):
- NAME = 'RELAY'
- VALUE = 3
-
-
class RelayCell(CircuitCell):
"""
Command concerning a relay circuit.
@@ -684,264 +491,6 @@ class RelayCell(CircuitCell):
return stem.util._hash_attr(self, 'command_int', 'stream_id', 'digest', 'data', cache = True)
-# TODO: merge the below with the RelayCell
-
-class AlternateRelayCell(CircuitCell):
- """
- Command concerning a relay circuit.
-
- :var stem.client.datatype.RelayCommand command: command to be issued
- :var int command_int: integer value of our command
- :var bytes data: payload of the cell
- :var int recognized: zero if cell is decrypted, otherwise mostly non-zero
- (can rarely be zero)
- :var int digest: running digest held with the relay
- :var int stream_id: specific stream this concerns
- """
-
- NAME = 'RELAY'
- VALUE = 3
- IS_FIXED_SIZE = True
- CANNOT_DIRECTLY_UNPACK = True
-
- def __init__(self, circ_id, command, data, digest = 0, stream_id = 0, recognized = 0, unused = b''):
- digest = RelayCell._coerce_digest(digest)
-
- super(RelayCell, self).__init__(circ_id, unused)
- self.command, self.command_int = RelayCommand.get(command)
- self.recognized = recognized
- self.stream_id = stream_id
- self.digest = digest
- self.data = str_tools._to_bytes(data)
-
- if digest == 0:
- if not stream_id and self.command in STREAM_ID_REQUIRED:
- raise ValueError('%s relay cells require a stream id' % self.command)
- elif stream_id and self.command in STREAM_ID_DISALLOWED:
- raise ValueError('%s relay cells concern the circuit itself and cannot have a stream id' % self.command)
-
- @classmethod
- def decrypt(link_protocol, content, digest, key):
- """
- Parse the given content as an encrypted RELAY cell.
- """
-
- # TODO: Fill in the above pydocs, deduplicate with the other decrypt
- # method, yadda yadda. Starting with a minimal stub to see if this makes
- # the Circuit class better. I'll circle back to clean up this module if it
- # works.
-
- if len(content) != link_protocol.fixed_cell_length:
- raise stem.ProtocolError('RELAY cells should be %i bytes, but received %i' % (link_protocol.fixed_cell_length, len(content)))
-
- circ_id, content = link_protocol.circ_id_size.pop(content)
- command, payload = Size.CHAR.pop(content)
-
- if command != RelayCell.VALUE:
- raise stem.ProtocolError('Cannot decrypt as a RELAY cell. This had command %i instead.' % command)
-
- key = copy.copy(key)
- decrypted = key.update(payload)
-
- # TODO: Integrate with check_digest() and flag for integrating if we're
- # fully decrypted. For the moment we only handle direct responses (ie. all
- # the cells we receive can be fully decrypted) but if we attempt to support
- # relaying we'll need to pass along cells we can only partially decrypt.
-
- return RelayCell._unpack(decrypted, circ_id, link_protocol), key, digest
-
- @staticmethod
- def _coerce_digest(digest):
- """
- Coerce any of HASH, str, int into the proper digest type for packing
-
- :param HASH,str,int digest: digest to be coerced
- :returns: digest in type appropriate for packing
-
- :raises: **ValueError** if input digest type is unsupported
- """
-
- if 'HASH' in str(type(digest)):
- # Unfortunately hashlib generates from a dynamic private class so
- # isinstance() isn't such a great option. With python2/python3 the
- # name is 'hashlib.HASH' whereas PyPy calls it just 'HASH'.
-
- digest_packed = digest.digest()[:RELAY_DIGEST_SIZE.size]
- digest = RELAY_DIGEST_SIZE.unpack(digest_packed)
- elif stem.util._is_str(digest):
- digest_packed = digest[:RELAY_DIGEST_SIZE.size]
- digest = RELAY_DIGEST_SIZE.unpack(digest_packed)
- elif stem.util._is_int(digest):
- pass
- else:
- raise ValueError('RELAY cell digest must be a hash, string, or int but was a %s' % type(digest).__name__)
-
- return digest
-
- def pack(self, link_protocol):
- payload = self.pack_payload()
-
- return RelayCell._pack(link_protocol, payload, unused = b'', circ_id = self.circ_id)
-
- @classmethod
- def _unpack(cls, content, circ_id, link_protocol):
- command, recognized, stream_id, digest, data_len, data, unused = RelayCell._unpack_payload(content)
-
- if len(data) != data_len:
- raise ValueError('%s cell said it had %i bytes of data, but only had %i' % (cls.NAME, data_len, len(data)))
-
- return RelayCell(circ_id, command, data, digest, stream_id, recognized, unused)
-
- @staticmethod
- def _unpack_payload(content):
- """
- Directly interpret the payload without any validation.
-
- :param bytes content: cell payload
-
- :returns: (command, recognized, stream_id, digest, data_len, data, unused) tuple
- """
-
- command, content = Size.CHAR.pop(content)
- recognized, content = Size.SHORT.pop(content) # 'recognized' field
- stream_id, content = Size.SHORT.pop(content)
- digest, content = Size.LONG.pop(content)
- data_len, content = Size.SHORT.pop(content)
- data, unused = split(content, data_len)
-
- return command, recognized, stream_id, digest, data_len, data, unused
-
- def apply_digest(self, digest, prep_cell = True):
- """
- Calculates, updates, and applies the digest to the cell payload,
- returning a new (cell, digest) tuple.
-
- :param HASH digest: running digest held with the relay
- :param bool prep_cell: preps the cell payload according to the spec, if
- **True** (default)
- if **False**, the digest will be calculated as-is, namely:
- * the 'recognized' field will not be set to 0,
- * the digest field will not be set to 0,
- * and any 'unused' padding will be taken as-is.
- Use **False** with caution.
-
- :returns: (:class:`~stem.client.cell.RelayCell`, HASH) tuple of object
- copies updated as follows:
- * digest: updated via digest.update(payload)
- * RelayCell: a copy of self, with the following updates:
- * RelayCell.recognized: set to 0, if prep_cell is **True**
- * RelayCell.digest: updated with the calculated digest
- * RelayCell.unused: treated as padding and overwritten, if prep_cell
- is **True**
- """
-
- if prep_cell:
- new_cell_recognized = 0
- new_cell_digest = 0
- new_cell_unused = b''
- else:
- new_cell_recognized = self.recognized
- new_cell_digest = self.digest
- new_cell_unused = self.unused
-
- new_digest = digest.copy()
- new_cell = RelayCell(self.circ_id, self.command, self.data, digest = new_cell_digest, stream_id = self.stream_id, recognized = new_cell_recognized, unused = new_cell_unused)
-
- payload_without_updated_digest = new_cell.pack_payload()
- new_digest.update(payload_without_updated_digest)
- new_cell.digest = RelayCell._coerce_digest(new_digest)
-
- return new_cell, new_digest
-
- def encrypt(self, link_protocol, digest, encryptor, **kwargs):
- """
- Preps a cell payload, including calculating digest, and encrypts it,
- returning a new (RawRelayCell, digest, encryptor) tuple.
-
- The method name is technically a misnomer, as it also preps cell payload
- and applies the digest, prior to encrypting. However, these operations
- are defined per the spec as required for RELAY cells, and ...
- (1) it is a natural mental extension to include them here;
- (2) it would be a bit pointless to require method consumers to manually
- call both, for pedantry.
-
- :param int link_protocol: link protocol version
- :param HASH digest: running digest held with the relay
- :param cryptography.hazmat.primitives.ciphers.CipherContext encryptor:
- running stream cipher encryptor held with the relay
-
- :param bool prep_cell: (optional, defaults to **True**) refer to
- :func:`~stem.client.cell.RelayCell.apply_digest`
-
- :returns: (bytes, HASH, CipherContext)
- tuple of object copies updated as follows:
- * bytes: encrypted cell payload
- * digest: updated via digest.update(payload)
- * encryptor: updated via encryptor.update(payload_with_digest)
- """
-
- unencrypted_cell, new_digest = self.apply_digest(digest, **kwargs)
- new_encryptor = copy.copy(encryptor)
- encrypted_payload = new_encryptor.update(unencrypted_cell.pack_payload())
- encrypted_cell = RawRelayCell(unencrypted_cell.circ_id, encrypted_payload)
-
- return encrypted_cell.pack(link_protocol), new_digest, new_encryptor
-
- def pack_payload(self, **kwargs):
- """
- Convenience method for running
- :func:`~stem.client.cell.RelayCell._pack_payload` on self.
-
- :param bool pad_remaining: (optional, defaults to **True**) pads up to
- payload size if **True**
-
- :returns: **bytes** with the packed payload
- """
-
- return RelayCell._pack_payload(self.command_int, self.recognized, self.stream_id, self.digest, len(self.data), self.data, self.unused, **kwargs)
-
- @staticmethod
- def _pack_payload(command_int, recognized, stream_id, digest, data_len, data, unused = b'', pad_remainder = True):
- """
- Directly pack the payload without any validation beyond Size constraints.
-
- :param int command_int: integer value of our command
- :param int recognized: zero if cell is decrypted, otherwise mostly non-zero
- (can rarely be zero)
- :param int stream_id: specific stream this concerns
- :param HASH,str,int digest: running digest held with the relay
- :param int data_len: length of body data
- :param bytes data: body data of the cell
- :param bytes unused: padding bytes to include after data
- :param bool pad_remaining: pads up to payload size if **True**
-
- :returns: **bytes** with the packed payload
- """
-
- payload = bytearray()
- payload += Size.CHAR.pack(command_int)
- payload += Size.SHORT.pack(recognized)
- payload += Size.SHORT.pack(stream_id)
- payload += Size.LONG.pack(RelayCell._coerce_digest(digest))
- payload += Size.SHORT.pack(data_len)
- payload += data
- payload += unused
-
- if len(payload) > FIXED_PAYLOAD_LEN:
- raise ValueError('Payload is too large (%i bytes), must not be more than %i.' % (len(payload), FIXED_PAYLOAD_LEN))
-
- if pad_remainder:
- # right now, it is acceptable to pad the remaining portion with ZEROs instead of random
- # this is done due to threat model and simplifying some implementation
- # however: in the future (TODO), this may become against the spec; see prop 289
- payload += ZERO * (FIXED_PAYLOAD_LEN - len(payload))
-
- return bytes(payload)
-
- def __hash__(self):
- return stem.util._hash_attr(self, 'command_int', 'stream_id', 'digest', 'data', cache = True)
-
-
class DestroyCell(CircuitCell):
"""
Closes the given circuit.
diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py
index 74d664d4..22843799 100644
--- a/test/unit/client/cell.py
+++ b/test/unit/client/cell.py
@@ -14,9 +14,6 @@ from stem.client.cell import (
FIXED_PAYLOAD_LEN,
Cell,
PaddingCell,
- BaseRelayCell,
- RawRelayCell,
- AlternateRelayCell,
RelayCell,
DestroyCell,
CreateFastCell,
@@ -95,24 +92,20 @@ AUTH_CHALLENGE_CELLS = {
class TestCell(unittest.TestCase):
def test_by_name(self):
- for (expected_class, name, value, is_fixed_size) in ((NetinfoCell, 'NETINFO', 8, True), (RawRelayCell, 'RELAY', 3, True)):
- cls = Cell.by_name(name)
- self.assertEqual(expected_class, cls)
- self.assertEqual(name, cls.NAME)
- self.assertEqual(value, cls.VALUE)
- self.assertEqual(is_fixed_size, cls.IS_FIXED_SIZE)
+ cls = Cell.by_name('NETINFO')
+ self.assertEqual('NETINFO', cls.NAME)
+ self.assertEqual(8, cls.VALUE)
+ self.assertEqual(True, cls.IS_FIXED_SIZE)
self.assertRaises(ValueError, Cell.by_name, 'NOPE')
self.assertRaises(ValueError, Cell.by_name, 85)
self.assertRaises(ValueError, Cell.by_name, None)
def test_by_value(self):
- for (expected_class, name, value, is_fixed_size) in ((NetinfoCell, 'NETINFO', 8, True), (RawRelayCell, 'RELAY', 3, True)):
- cls = Cell.by_value(value)
- self.assertEqual(expected_class, cls)
- self.assertEqual(name, cls.NAME)
- self.assertEqual(value, cls.VALUE)
- self.assertEqual(is_fixed_size, cls.IS_FIXED_SIZE)
+ cls = Cell.by_value(8)
+ self.assertEqual('NETINFO', cls.NAME)
+ self.assertEqual(8, cls.VALUE)
+ self.assertEqual(True, cls.IS_FIXED_SIZE)
self.assertRaises(ValueError, Cell.by_value, 'NOPE')
self.assertRaises(ValueError, Cell.by_value, 85)
@@ -195,39 +188,7 @@ class TestCell(unittest.TestCase):
self.assertEqual(b'', cell.unused) # always empty
self.assertEqual(cell_bytes, cell.pack(link_protocol))
- def test_base_relay_cell(self):
- arbitrary_circ_id = 123
- even_more_arbitrary_link_protocol = 1234
-
- cell = BaseRelayCell(arbitrary_circ_id, RANDOM_PAYLOAD)
- self.assertEqual(RANDOM_PAYLOAD, cell.payload)
- self.assertEqual(arbitrary_circ_id, cell.circ_id)
- self.assertEqual(True, cell.IS_FIXED_SIZE)
-
- # Cell.unpack not reachable - won't be tested
- # but we can at least directly test _unpack, although it's a pretty simple method
- cell_2 = BaseRelayCell._unpack(RANDOM_PAYLOAD, arbitrary_circ_id, even_more_arbitrary_link_protocol)
- self.assertEqual(cell, cell_2)
-
- # pack not possible, but easily callable
- # lots of things cause a ValueError, so this check isn't very specific, but the wording comes from Size and so isn't under the purview of this unit
- self.assertRaises(ValueError, cell.pack, even_more_arbitrary_link_protocol)
-
- # check other values and inequality
- for (circ_id, payload) in ((arbitrary_circ_id, ZERO * FIXED_PAYLOAD_LEN), (arbitrary_circ_id + 1, RANDOM_PAYLOAD)):
- unequal_cell = BaseRelayCell(circ_id, payload)
- self.assertEqual(payload, unequal_cell.payload)
- self.assertNotEqual(cell, unequal_cell)
-
- # invalid constructions
- self.assertRaisesWith(ValueError, 'Relay cells require a payload', BaseRelayCell, arbitrary_circ_id, None)
- expected_message_format = 'Payload should be %i bytes, but was ' % FIXED_PAYLOAD_LEN + '%i'
- for payload_len in (FIXED_PAYLOAD_LEN - 1, FIXED_PAYLOAD_LEN + 1):
- self.assertRaisesWith(ValueError, expected_message_format % payload_len, BaseRelayCell, arbitrary_circ_id, ZERO * payload_len)
-
def test_relay_cell(self):
- self.assertEquals(True, AlternateRelayCell.CANNOT_DIRECTLY_UNPACK)
-
for cell_bytes, (command, command_int, circ_id, stream_id, data, digest, unused, link_protocol) in RELAY_CELLS.items():
if not unused.strip(ZERO):
self.assertEqual(cell_bytes, RelayCell(circ_id, command, data, digest, stream_id).pack(link_protocol))
@@ -236,12 +197,7 @@ class TestCell(unittest.TestCase):
self.assertEqual(cell_bytes, RelayCell(circ_id, command, data, digest, stream_id, unused = unused).pack(link_protocol))
self.assertEqual(cell_bytes, RelayCell(circ_id, command_int, data, digest, stream_id, unused = unused).pack(link_protocol))
- # first unpack via RawRelayCell, then interpret into RelayCell
- raw_cell = Cell.pop(cell_bytes, link_protocol)[0]
- self.assertEqual(circ_id, raw_cell.circ_id)
- self.assertEqual(cell_bytes[-FIXED_PAYLOAD_LEN:], raw_cell.payload)
-
- cell = raw_cell.interpret_cell()
+ cell = Cell.pop(cell_bytes, link_protocol)[0]
self.assertEqual(circ_id, cell.circ_id)
self.assertEqual(command, cell.command)
self.assertEqual(command_int, cell.command_int)
@@ -251,7 +207,6 @@ class TestCell(unittest.TestCase):
self.assertEqual(unused, cell.unused)
self.assertEqual(cell_bytes, cell.pack(link_protocol))
- self.assertEqual(cell_bytes, raw_cell.pack(link_protocol))
digest = hashlib.sha1(b'hi')
self.assertEqual(3257622417, RelayCell(5, 'RELAY_BEGIN_DIR', '', digest, 564346860).digest)
@@ -271,9 +226,7 @@ class TestCell(unittest.TestCase):
ZERO * 498, # data
))
- # TODO - temporarily, we hack the interim tests by unpacking info via RawRelayCell
- raw_cell = Cell.pop(mismatched_data_length_bytes, 2)[0]
- self.assertRaisesWith(ValueError, 'RELAY cell said it had 65535 bytes of data, but only had 498', RelayCell._unpack, raw_cell.payload, raw_cell.circ_id, 2)
+ self.assertRaisesWith(ValueError, 'RELAY cell said it had 65535 bytes of data, but only had 498', Cell.pop, mismatched_data_length_bytes, 2)
def test_destroy_cell(self):
for cell_bytes, (circ_id, reason, reason_int, unused, link_protocol) in DESTROY_CELLS.items():
diff --git a/test/unit/client/size.py b/test/unit/client/size.py
index 733db0f4..dcd5fea3 100644
--- a/test/unit/client/size.py
+++ b/test/unit/client/size.py
@@ -25,7 +25,6 @@ class TestSize(unittest.TestCase):
self.assertEqual(b'\x00\x00\x00\x00\x00\x00\x00\x12', Size.LONG_LONG.pack(18))
self.assertRaisesWith(ValueError, 'Size.pack encodes an integer, but was a str', Size.CHAR.pack, 'hi')
-
self.assertRaisesWith(ValueError, 'Packed values must be positive (attempted to pack -1 as a CHAR)', Size.CHAR.pack, -1)
bad_size = Size('BAD_SIZE', 1, '!H')
