Skip to content
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

urlglob does not allow dashes in IPv6 zone ID #5576

Closed
puckipedia opened this issue Jun 17, 2020 · 7 comments
Closed

urlglob does not allow dashes in IPv6 zone ID #5576

puckipedia opened this issue Jun 17, 2020 · 7 comments

Comments

@puckipedia
Copy link

It seems that urlglob's IPv6 parsing is less lenient than the actual URL parsing is, it doesn't allow for dashes..

I did this

marisa ~> curl -v 'http://[fe80::1%foo-bar]/`
curl: (3) bad range in URL position 9:
http://[fe80::1%foo-bar]/
        ^

I expected the following

marisa ~> curl -gv 'http://[fe80::1%foo-bar]/'
*   Trying fe80::1:80...

curl/libcurl version

curl 7.70.0 (x86_64-pc-linux-gnu) libcurl/7.70.0 OpenSSL/1.1.1g zlib/1.2.11 libssh2/1.9.0 nghttp2/1.40.0
Release-Date: 2020-04-29
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz NTLM NTLM_WB SPNEGO SSL TLS-SRP UnixSockets

operating system

Linux marisa 5.6.15 #1-NixOS SMP Wed May 27 15:48:31 UTC 2020 x86_64 GNU/Linux

@dfandrich
Copy link
Contributor

dfandrich commented Jun 17, 2020 via email

@puckipedia
Copy link
Author

marisa ~> curl -v 'http://[fe80::1%25foo-bar]/'
curl: (3) bad range in URL position 9:
http://[fe80::1%25foo-bar]/

And either way, RFC6874 Section 3 suggests parsers be liberal in accepting non-percent-escaped zone IDs in IPv6 literals.

@dfandrich
Copy link
Contributor

dfandrich commented Jun 17, 2020 via email

@puckipedia
Copy link
Author

The only reference to globbing and IPv6 I could find is https://ec.haxx.se/cmdline/cmdline-globbing, which says:

The globbing uses the reserved symbols [] and {} for this, symbols that normally cannot be part of a legal URL (except for numerical IPv6 addresses but curl handles them fine anyway). If the globbing gets in your way, disable it with -g, --globoff.

now, I can see why this is occuring, but src/tool_urlglob.c seems to imply that IPv6 addresses are not parsed as ranges by the globbing mechanism:

/* skip over IPv6 literals and [] */

If IPv6 literal addresses in URLs shouldn't be supported without -g, why is this rule not applied consistently?

@bagder bagder added the URL label Jun 17, 2020
@bagder
Copy link
Member

bagder commented Jun 17, 2020

Look like the peek_ipv6 function needs to be fixed to deal with zone id better:

curl/src/tool_urlglob.c

Lines 322 to 350 in 8bc25c5

static bool peek_ipv6(const char *str, size_t *skip)
{
/*
* Scan for a potential IPv6 literal.
* - Valid globs contain a hyphen and <= 1 colon.
* - IPv6 literals contain no hyphens and >= 2 colons.
*/
size_t i = 0;
size_t colons = 0;
if(str[i++] != '[') {
return FALSE;
}
for(;;) {
const char c = str[i++];
if(ISALNUM(c) || c == '.' || c == '%') {
/* ok */
}
else if(c == ':') {
colons++;
}
else if(c == ']') {
*skip = i;
return colons >= 2 ? TRUE : FALSE;
}
else {
return FALSE;
}
}
}

bagder added a commit that referenced this issue Jun 18, 2020
... not a "glob". Now done by passing the supposed host to the URL
parser which supposedly will do a better job at identifying "real"
numerical IPv6 addresses.

Reported-by: puckipedia on github
Fixes #5576
@bagder
Copy link
Member

bagder commented Jun 18, 2020

@puckipedia do you have the ability to try out my patch for this?

@puckipedia
Copy link
Author

Looks like it works for me, thank you very much!

marisa ~> $result/bin/curl -v 'http://[fe80::1%foo-bar]/'
*   Trying fe80::1:80...
* Connected to fe80::1 (fe80::1) port 80 (#0)

@bagder bagder closed this as completed in fa4fbc5 Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants