[PATCH] remove zone identifier for IPv6 link-local addresses

[Fabian Hugelshofer]

Denys Vlasenko wrote:
> On Sun, Jun 28, 2009 at 7:28 PM, Fabian
> Hugelshofer<hugelshofer2006 at gmx.ch> wrote:
>> Below you find the patch including Rob's suggestions.
>>
>> Fabian
> 
> strip_ipv6_scope can be simpler:

Yes, I agree.

> static void strip_ipv6_scope(char *host)
> {
>         char *scope, *cp;
> 
>         scope = strchr(host, '%');
>         if (!scope)
>                 return;
> 
>         /* Remove the IPv6 zone identifier from the host address */
>         /* Ugly IPv6 parsing like in str2sockaddr() */
>         if (host[0] == '[') {
>                 cp = strchr(host, ']');
>                 if (!cp || (cp[1] != ':' && cp[1] != '\0')) {
>                         /* malformed address (not "[xx]:nn" or "[xx]") */
>                         return;
>                 }
>                 if (scope > cp)
>                         return;
>                 /* cp points to "]...", scope points to "%eth0]..." */

I think this "if (scope > cp)" could be removed. There should be no '%'
after the ']' as it seems that %-encoding in the host/port-part of an
URL isn't supported anyway.

> I propose the following attached patch. It is against newer git.

Dva changed the way I called the strip function and missed out a small
detail. From dva's patch:

if (use_proxy) {
	proxy = getenv(target.is_ftp ? "ftp_proxy" : "http_proxy");
	if (proxy && proxy[0]) {
		parse_url(proxy, &server);
	} else {
		use_proxy = 0;
	}
} else {
	server.port = target.port;
	if (ENABLE_FEATURE_IPV6) {
		server.host = xstrdup(target.host);
	} else {
		server.host = target.host;
	}
}

This is wrong, as use_proxy (that is true by default) can get set to
false within the condition. In this case the server parameters would not
be set at all. The server assignment stuff needs a separate condition.
See the version in the attached patch.


> Please test, and let the list know what commands do you test,
> do you use $http_proxy variable etc... Like:
> 
> wget http://::1&lo/path
> wget http://[::1&lo]:80/path
> ...

I tested the patch with the fixed proxy condition with http_proxy unset.
I successfully tested the following commands:

wget http://[::1]/path
wget http://::1/path
wget http://[fe80::1%eth0]/path
wget http://[fe80::1%eth0]:80/path
wget http://fe80::1%eth0/path


> Code impact is a bit big:
> 
> # make bloatcheck
> function                                             old     new   delta
> wget_main                                           2331    2433    +102

IMHO it would be possible to remove support for addresses without
[enclosure] in the strip function. According to RFC3986
(http://tools.ietf.org/html/rfc3986#section-3.2.2) IPv6 addresses need
to be enclosed in URIs. str2sockaddr() only supports both versions as it
is not exclusively called with addresses from URIs.

Attached you find a 9.patch that fixes the use_proxy condition and a
10.patch that further removes the parts mentioned above.

Fabian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 9.patch
Type: text/x-diff
Size: 2924 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20090630/e69d58c5/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 10.patch
Type: text/x-diff
Size: 2729 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20090630/e69d58c5/attachment-0001.bin>