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

Add support for IPv6 literal hostname in url #181

Closed
wants to merge 2 commits into from
Closed

Add support for IPv6 literal hostname in url #181

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 30, 2018

Commit bfd6e88:

Axel was previously unable to parse ipv6 address as hostname of url.
This has been fixed, conforming to RFC 2732

Closes #169

Commit 8de7c1a:

Axel did not add port number to the absolute url in the GET request to the proxy if it is not the default port. This has been fixed in this commit.

@ismaell please review my pull requests.

Shankar added 2 commits October 31, 2018 12:09
Axel was previously unable to parse ipv6 address as hostname of url.
This has been fixed, conforming to RFC 2732

Closes #169

Signed-off-by: Shankar <shankar_m@protonmail.com>
Axel failed to add port number to the absolute-url in the request-target
message to the proxy if the port number is not the default value to that
specific protocol. This has been fixed.

Signed-off-by: Shankar <shankar_m@protonmail.com>
Copy link
Member

@ismaell ismaell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be improved.

if (strrchr(conn->host, '@') != NULL) {
strncpy(conn->user, conn->host, sizeof(conn->user) - 1);
if (strrchr(url, '@') != NULL) {
strncpy(conn->user, url, sizeof(conn->user) - 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please replace strncpy with strlcpy.

if (strrchr(conn->host, '@') != NULL) {
strncpy(conn->user, conn->host, sizeof(conn->user) - 1);
if (strrchr(url, '@') != NULL) {
strncpy(conn->user, url, sizeof(conn->user) - 1);
i = strrchr(conn->user, '@');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be accomplished by reducing the size to be copied.

i = strrchr(conn->user, '@');
*i = 0;
strncpy(conn->host, i + 1, sizeof(conn->host) - 1);
strncpy(url, i + 1, sizeof(url) - 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

// Look for IPv6 literal hostname
if (*url == '[') {
strncpy(conn->host, url + 1, sizeof(conn->host) - 1);
if ((i = strrchr(conn->host, ']')) != NULL) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strrchr + strlcpy instead of fixing up the result.


if (inet_pton(AF_INET6, hostname, buf))
return 1;
return 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be

    return hostname && 1 == inet_pton(AF_INET6, hostname, buf);

strcpy(string, scheme_from_proto(conn->proto));

if (*conn->user != 0 && strcmp(conn->user, "anonymous") != 0)
sprintf(string + strlen(string), "%s:%s@",
conn->user, conn->pass);

sprintf(string + strlen(string), "%s:%i%s%s",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL formatting should be split to somewhere else so it can be shared between conn.c and http.c

@ismaell ismaell added this to the v2.17 milestone Apr 15, 2019
@ismaell ismaell self-assigned this Apr 15, 2019
@ismaell
Copy link
Member

ismaell commented Apr 15, 2019

@shankar ping.

@ismaell
Copy link
Member

ismaell commented Apr 22, 2019

Merged with minor edits.

@ismaell ismaell closed this Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IPv6 Support
1 participant