Repository navigation
fix: support IPv6 addresses in the local Appium service URL - #2495
RaphaelFakhri wants to merge 1 commit into
Conversation
mykola-mokhnach
left a comment
There was a problem hiding this comment.
AppiumDriverLocalService.java:129:replaceHostdoes a regexreplaceFirston the whole URL string. The new IPv6 path only works because"http://[::]:4777/".replaceFirst("::", "::1")happens to hit the host.oldHostisn't quoted, so the dots in0.0.0.0are regex wildcards, andnewHostisn't passed throughMatcher.quoteReplacement. Would building the URL fromgetProtocol()/getHost()/getPort()be safer?
| this.startupTimeout = startupTimeout; | ||
| this.url = new URL(String.format(URL_MASK, ipAddress, nodeJSPort)); | ||
| // IPv6 literals must be enclosed in square brackets to be a valid URL host | ||
| String host = ipAddress.contains(":") && !ipAddress.startsWith("[") ? "[" + ipAddress + "]" : ipAddress; |
There was a problem hiding this comment.
The bracket-wrapping check (contains(":") and not starting with [) misses some IPv6 forms:
- A zone id like
fe80::1%en0becomes[fe80::1%en0], but%must be encoded as%25in a URL, so the host is mis-parsed or the connection fails. - If a user passes a bracketed
[::1]towithIPAddress, the URL is correct, butcreateArgspasses the literal[::1]to Appium's--address, which Node rejects. The two code paths disagree. Should the address be normalized once and the unbracketed form used for--address?
|
|
||
| private URL fixBroadcastAddresses(URL url) { | ||
| var host = url.getHost(); | ||
| // IPv6 hosts are returned in square brackets |
There was a problem hiding this comment.
Only the exact string :: is treated as the IPv6 any-address. 0:0:0:0:0:0:0:0 and ::0 are equivalent wildcard binds but won't be rewritten to loopback. ping then connects to the unspecified address, which some OSes block, so start() times out even though the server is up.
|
|
||
| private URL fixBroadcastAddresses(URL url) { | ||
| var host = url.getHost(); | ||
| // IPv6 hosts are returned in square brackets |
There was a problem hiding this comment.
Nit: "^\\[(.*)]$" is recompiled on every ping. host.startsWith("[") && host.endsWith("]") with substring does the same job, or use a precompiled Pattern. It could also sit in one helper next to the bracket-wrapping at line 87.
| assertFalse(service.isRunning()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
The new tests only check URL formatting. Nothing exercises fixBroadcastAddresses or the bracket-stripping in the ping path. If [::] stops being rewritten to ::1, no test fails. An already-bracketed [::1] input and a zone-id input are also untested.
Change list
AppiumDriverLocalServiceencloses an IPv6 literal (for example::or::1) in square brackets when it builds the server URL. Before this change,build()failed withMalformedURLException.BROADCAST_IP6_ADDRESS(::) is pinged on[::1]as intended.AppiumServiceBuilderTestcover::1andBROADCAST_IP6_ADDRESS.Types of changes
Details
AppiumServiceBuilderexposesBROADCAST_IP6_ADDRESS, but passing it, or any other IPv6 address, towithIPAddressbreaksbuild():The service formats the URL as
http://%s:%d/, which produceshttp://:::4723/. With brackets the URL becomeshttp://[::]:4723/. The--addressvalue sent to the server stays unbracketed, because the server expects a plain address.fixBroadcastAddressescomparedURL.getHost()with::, butgetHost()returns[::]for IPv6 hosts, so the IPv6 branch could never match. It now strips the brackets before the comparison.