Skip to content

Commit

Permalink
IDN host names: Remove the port number before converting to ACE
Browse files Browse the repository at this point in the history 
Closes #596
  • Loading branch information
@mkauf @bagder
mkauf authored and bagder committed Jan 10, 2016
1 parent 036c465 commit 5d7c937
Showing 1 changed file with 7 additions and 7 deletions.
14 changes: 7 additions & 7 deletions lib/url.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5651,13 +5651,6 @@ static CURLcode create_conn(struct SessionHandle *data,
if((conn->given->flags&PROTOPT_SSL) && conn->bits.httpproxy)
conn->bits.tunnel_proxy = TRUE;

/*************************************************************
* IDN-fix the hostnames
*************************************************************/
fix_hostname(data, conn, &conn->host);
if(conn->proxy.name && *conn->proxy.name)
fix_hostname(data, conn, &conn->proxy);

/*************************************************************
* Figure out the remote port number and fix it in the URL
*************************************************************/
Expand All @@ -5674,6 +5667,13 @@ static CURLcode create_conn(struct SessionHandle *data,
if(result)
goto out;

/*************************************************************
* IDN-fix the hostnames
*************************************************************/
fix_hostname(data, conn, &conn->host);
if(conn->proxy.name && *conn->proxy.name)
fix_hostname(data, conn, &conn->proxy);

/*************************************************************
* Setup internals depending on protocol. Needs to be done after
* we figured out what/if proxy to use.
Expand Down

4 comments on commit 5d7c937

@gvanem
Copy link
Contributor

@gvanem gvanem commented on 5d7c937 Feb 5, 2016

Choose a reason for hiding this comment

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

I suspect there is a bug (related to this or something else) when libcurl is built with WinIDN.

Regarding the call to curl_win32_idn_to_ascii() in url.c:

    char *ace_hostname = NULL;
    int rc = curl_win32_idn_to_ascii(host->name, &ace_hostname);
    if(rc == 0)
      infof(data, "Failed to convert %s to ACE;\n",
            host->name);
    else {
     ...

And in idn_win32.c + curl_win32_idn_to_ascii():

It is possible that curl_win32_idn_to_ascii() returns 1 and ace_hostname still be NULL. Since Curl_convert_UTF8_to_wchar(in) can fail (depending on code-page and what not).

What should happen in url.c then? Or is a host->name == NULL handled correctly thereafter? I fail to follow all the details.

@gvanem
Copy link
Contributor

@gvanem gvanem commented on 5d7c937 Feb 5, 2016

Choose a reason for hiding this comment

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

In other words, I feel url.c should be patched as:

--- a/url.c  2016-02-05 07:58:13
+++ b/url.c  2016-02-05 17:27:21
@@ -3790,10 +3790,13 @@
    *************************************************************/
     char *ace_hostname = NULL;
     int rc = curl_win32_idn_to_ascii(host->name, &ace_hostname);
-    if(rc == 0)
+    if(rc == 0 || !ace_hostname)
       infof(data, "Failed to convert %s to ACE;\n",
             host->name);

@mkauf
Copy link
Contributor Author

@mkauf mkauf commented on 5d7c937 Feb 5, 2016

Choose a reason for hiding this comment

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

You are right, host->name must not be NULL after curl_win32_idn_to_ascii() has been called.

I think that curl_win32_idn_to_ascii() should really return 0 if it has failed to convert the host name. I have created #637 for this.

@jay
Copy link
Member

@jay jay commented on 5d7c937 Feb 5, 2016

Choose a reason for hiding this comment

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

It is possible that curl_win32_idn_to_ascii() returns 1 and ace_hostname still be NULL

That's the issue to address. Discussion moved to #637

Please sign in to comment.