-
Notifications
You must be signed in to change notification settings - Fork 112
Enhance transports #760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhance transports #760
Conversation
zxzxwu
commented
Aug 18, 2025
- Support IPv6 schema
- Add transport integration tests
- Add UNIX socket server
|
I lost my access to this repo due to recent corp policy change 😓 Requesting access. |
bumble/transport/android_emulator.py
Outdated
| mode = param.split('=')[1] | ||
| elif ':' in param: | ||
| server_host, server_port = param.split(':') | ||
| server_host, server_port = param.rsplit(':', maxsplit=1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To properly support IPv6 addresses, we need a more robust parser to separate the host and port. (':' in param isn't sufficient, since you could have an IPv6 address that has a :, but not followed by :<port>). I think we should factor out a shared function (maybe in common.py?) that knows how to parse IPv6 addresses with ports of the form [address]:port like [2001:db8:85a3:8d3:1319:8a2e:370:7348]:8000 (i.e enclosing the address in [] is mandatory when followed by a port number).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though it's not very complicated to do this, I am wondering if that's necessary - because we don't allow any schema without a complete address:port pair, so even if params and ':' in params[0] returns a false positive, it will still fail later such as
# python apps/controller_info.py android-netsim::::
File "/usr/local/google/home/joshwu/projects/bumble/bumble/transport/android_netsim.py", line 447, in open_android_netsim_transport
port = int(port_str)
ValueError: invalid literal for int() with base 10: ''
# or
grpc.aio._call.AioRpcError: <AioRpcError of RPC that terminated with:
status = StatusCode.UNAVAILABLE
details = "DNS resolution failed for :::1: C-ares status is not ARES_SUCCESS qtype=AAAA name=:::1 is_balancer=0: Domain name not found"
debug_error_string = "UNKNOWN:Error received from peer {grpc_status:14, grpc_message:"DNS resolution failed for :::1: C-ares status is not ARES_SUCCESS qtype=AAAA name=:::1 is_balancer=0: Domain name not found"}"
>
Also this might violate metadata schema defined in open_transport
scheme, *tail = name.split(':', 1)
spec = tail[0] if tail else None
metadata = None
if spec:
# Metadata may precede the spec
if spec.startswith('['):
metadata_str, *tail = spec[1:].split(']')
spec = tail[0] if tail else None
metadata = dict([entry.split('=') for entry in metadata_str.split(',')])There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, I found this section is totally not neceassary, since the grpc module directly parses the port, so we can remove it here.
For other transports, I think we should keep the current schema to maintain the compatibility with transport metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, we always have the port explicitly specified, so it's fine to split out the portion after the last :, and not require [] around IPv6 addresses. This will keep the metadata parser simple (if we needed, we could differentiate [foo=bar] metadata from [xx:yy:zz] IPv6, but we don't need to here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still, I found gRPC cannot parse such address, so as you said, we can heuristically distinguish between metadata and IPv6 address (in another PR).
4856ab5 to
11dd0a3
Compare
* Support IPv6 schema * Add transport integration tests * Add UNIX socket server