Skip to content

Commit

Permalink
pw_rpc: Fix if IDs are negative
Browse files Browse the repository at this point in the history
Service and method IDs are unsigned 32-bit integers. It seems that
these occasionally appear as negative values in the Python client.

We can't just modify .service_id and .method_id since the underlying
storage is still erroneously a signed type.
Let's just make it unsigned just before we use it!

See also, CL that yells about this problem:
Ia1bf3013940d995b88e65b569634fc0f7e66abbe

Bug: b/239712573, b/239224232
Change-Id: I5b78aa0804bdb1f1d87f725d85adb0cdfab4d218
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/102760
Reviewed-by: Wyatt Hepler <[email protected]>
Commit-Queue: Anthony DiGirolamo <[email protected]>
amstan authored and CQ Bot Account committed Jul 22, 2022
1 parent 0788554 commit f592cfa
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions pw_rpc/py/pw_rpc/client.py
Original file line number Diff line number Diff line change
@@ -548,16 +548,22 @@ def process_packet(self, pw_rpc_raw_packet_data: bytes, *impl_args,
def _look_up_service_and_method(
self, packet: RpcPacket,
channel_client: ChannelClient) -> PendingRpc:
# Protobuf is sometimes silly so the 32 bit python bindings return
# signed values from `fixed32` fields. Let's convert back to unsigned.
# b/239712573
service_id = packet.service_id & 0xffffffff
try:
service = self.services[packet.service_id]
service = self.services[service_id]
except KeyError:
raise ValueError(f'Unrecognized service ID {packet.service_id}')
raise ValueError(f'Unrecognized service ID {service_id}')

# See above, also for b/239712573
method_id = packet.method_id & 0xffffffff
try:
method = service.methods[packet.method_id]
method = service.methods[method_id]
except KeyError:
raise ValueError(
f'No method ID {packet.method_id} in service {service.name}')
f'No method ID {method_id} in service {service.name}')

return PendingRpc(channel_client.channel, service, method)

0 comments on commit f592cfa

Please sign in to comment.