commit b00100d73410850d245cb1d1666829748f41ad69 Author: Dave Rolek dmr-x@riseup.net Date: Mon Jul 9 19:11:16 2018 +0000

Include a cell's 'unused' portion prior to remaining padding

This fixes #26766, as well as makes the aforementioned failing DestroyCell test pass.

Also change a few tests that used _pack() directly, since the parameter order changed with keyword arguments. --- stem/client/cell.py | 21 ++++++++++++--------- test/unit/client/cell.py | 6 +++--- 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/stem/client/cell.py b/stem/client/cell.py index 83019621..d04c387e 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -178,7 +178,7 @@ class Cell(object): return cls._unpack(payload, circ_id, link_protocol), content

@classmethod - def _pack(cls, link_protocol, payload, circ_id = None): + def _pack(cls, link_protocol, payload, unused = b'', circ_id = None): """ Provides bytes that can be used on the wire for these cell attributes. Format of a properly packed cell depends on if it's fixed or variable @@ -215,9 +215,12 @@ class Cell(object): cell = bytearray() 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 += b'' if cls.IS_FIXED_SIZE else Size.SHORT.pack(len(payload) + len(unused)) cell += payload

+ # include the unused portion (typically from unpacking) + cell += unused + # pad fixed sized cells to the required length

if cls.IS_FIXED_SIZE: @@ -366,7 +369,7 @@ class RelayCell(CircuitCell): payload += Size.SHORT.pack(len(self.data)) payload += self.data

- return RelayCell._pack(link_protocol, bytes(payload), self.circ_id) + return RelayCell._pack(link_protocol, bytes(payload), self.unused, self.circ_id)

@classmethod def _unpack(cls, content, circ_id, link_protocol): @@ -403,7 +406,7 @@ class DestroyCell(CircuitCell): self.reason, self.reason_int = CloseReason.get(reason)

def pack(self, link_protocol): - return DestroyCell._pack(link_protocol, Size.CHAR.pack(self.reason_int), self.circ_id) + return DestroyCell._pack(link_protocol, Size.CHAR.pack(self.reason_int), self.unused, self.circ_id)

@classmethod def _unpack(cls, content, circ_id, link_protocol): @@ -436,7 +439,7 @@ class CreateFastCell(CircuitCell): self.key_material = key_material

def pack(self, link_protocol): - return CreateFastCell._pack(link_protocol, self.key_material, self.circ_id) + return CreateFastCell._pack(link_protocol, self.key_material, self.unused, self.circ_id)

@classmethod def _unpack(cls, content, circ_id, link_protocol): @@ -477,7 +480,7 @@ class CreatedFastCell(CircuitCell): self.derivative_key = derivative_key

def pack(self, link_protocol): - return CreatedFastCell._pack(link_protocol, self.key_material + self.derivative_key, self.circ_id) + return CreatedFastCell._pack(link_protocol, self.key_material + self.derivative_key, self.unused, self.circ_id)

@classmethod def _unpack(cls, content, circ_id, link_protocol): @@ -554,7 +557,7 @@ class NetinfoCell(Cell): for addr in self.sender_addresses: payload += addr.pack()

- return NetinfoCell._pack(link_protocol, bytes(payload)) + return NetinfoCell._pack(link_protocol, bytes(payload), self.unused)

@classmethod def _unpack(cls, content, circ_id, link_protocol): @@ -659,7 +662,7 @@ class CertsCell(Cell): self.certificates = certs

def pack(self, link_protocol): - return CertsCell._pack(link_protocol, Size.CHAR.pack(len(self.certificates)) + b''.join([cert.pack() for cert in self.certificates])) + return CertsCell._pack(link_protocol, Size.CHAR.pack(len(self.certificates)) + b''.join([cert.pack() for cert in self.certificates]), self.unused)

@classmethod def _unpack(cls, content, circ_id, link_protocol): @@ -710,7 +713,7 @@ class AuthChallengeCell(Cell): for method in self.methods: payload += Size.SHORT.pack(method)

- return AuthChallengeCell._pack(link_protocol, bytes(payload)) + return AuthChallengeCell._pack(link_protocol, bytes(payload), self.unused)

@classmethod def _unpack(cls, content, circ_id, link_protocol): diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index c67f4d9e..bf2b9275 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -129,15 +129,15 @@ class TestCell(unittest.TestCase): def test_circuit_id_validation(self): # only CircuitCell subclasses should provide a circ_id

- self.assertRaisesRegexp(ValueError, 'PADDING cells should not specify a circuit identifier', PaddingCell._pack, 5, b'', 12) + self.assertRaisesRegexp(ValueError, 'PADDING cells should not specify a circuit identifier', PaddingCell._pack, 5, b'', circ_id = 12)

# CircuitCell should validate its circ_id

- self.assertRaisesRegexp(ValueError, 'RELAY cells require a circuit identifier', RelayCell._pack, 5, b'', None) + self.assertRaisesRegexp(ValueError, 'RELAY cells require a circuit identifier', RelayCell._pack, 5, b'', circ_id = None)

for circ_id in (0, -1, -50): expected_msg = 'Circuit identifiers must a positive integer, not %s' % circ_id - self.assertRaisesRegexp(ValueError, expected_msg, RelayCell._pack, 5, b'', circ_id) + self.assertRaisesRegexp(ValueError, expected_msg, RelayCell._pack, 5, b'', circ_id = circ_id)

def test_unpack_for_new_link(self): expected_certs = (