Yeah, that's 100% my bad. I didn't think that my changes would be of any interest after you made the feature work, so I didn't put the effort in making a proper patch. I'll give you proper contributions from now on. As for base64, there is always the option of adding one more dependency - which I guess we are trying to avoid :) Marc-Andr? Laverdi?re-Papineau Researcher - e-Security Team TCS Innovation Labs On 06/19/2014 02:34 PM, David Woodhouse wrote: > On Thu, 2014-06-19 at 09:12 +0530, Marc-Andr? Laverdi?re wrote: >> >> I discovered yesterday how rusty I got at C... oh well. >> Here are some of the changes I did, straight from the "I don't know >> what I'm doing dept." > > Looks good; thanks. I'd actually already implemented the > username/password parsing for openconnect_set_http_proxy() but it was a > bit of a hack, just post-processing the "hostname" returned by the > standard URL parsing function and looking for the @ sign. > > You've done a much better job, making internal_parse_url() do it sanely > and propagating that API change ? and as an added bonus you've also made > it use the xmlParseUri() function instead of its own implementation. I > hadn't realised that was available. > > There are *lots* of wheels reinvented in the OpenConnect code, which > always makes me sad, but often it's not avoidable. This one *was* > avoidable, and well done for spotting that. > > (If you can find a suitable base64 encode/decode function that I can use > too, that'd make me even happier. OpenSSL gives me one, but not GnuTLS > or libxml2 AFAICT.) > > It took me a while to work out what you'd changed, though. Instead of > complete files (which were so large that the mailing list didn't even > let them through), it's best to send a patch. That way, I don't have to > spend time trying to work out which historical version of the code you > *started* with, to tell your changes from all the other changes which > have been made since then. > > Here's your work as a patch, which makes it easy to read and review > right here in the mail. When submitting a patch please also include a > Signed-Off-By: tag as described at > http://www.infradead.org/openconnect/contribute.html > > Thanks again. > > diff --git a/http.c b/http.c > index 323ff43..8b9d87f 100644 > --- a/http.c > +++ b/http.c > @@ -27,6 +27,7 @@ > #include <stdlib.h> > #include <stdio.h> > #include <stdarg.h> > +#include <libxml/uri.h> > #ifndef _WIN32 > #include <pwd.h> > #endif > @@ -662,69 +663,65 @@ out: > #endif /* !_WIN32 */ > } > > -int internal_parse_url(char *url, char **res_proto, char **res_host, > - int *res_port, char **res_path, int default_port) > +int internal_parse_url(char *url, char **res_proto, char **res_host, int *res_port, > + char **res_auth_uname, char **res_auth_pwd, char **res_path, int default_port) > { > - char *proto = url; > - char *host, *path, *port_str; > - int port; > - > - host = strstr(url, "://"); > - if (host) { > - *host = 0; > - host += 3; > - > - if (!strcasecmp(proto, "https")) > - port = 443; > - else if (!strcasecmp(proto, "http")) > - port = 80; > - else if (!strcasecmp(proto, "socks") || > - !strcasecmp(proto, "socks4") || > - !strcasecmp(proto, "socks5")) > - port = 1080; > - else > - return -EPROTONOSUPPORT; > - } else { > - if (default_port) { > - proto = NULL; > - port = default_port; > - host = url; > - } else > - return -EINVAL; > - } > - > - path = strchr(host, '/'); > - if (path) > - *(path++) = 0; > > - port_str = strrchr(host, ':'); > - if (port_str) { > - char *end; > - int new_port = strtol(port_str + 1, &end, 10); > - > - if (!*end) { > - *port_str = 0; > - port = new_port; > - } > - } > - > - if (res_proto) > - *res_proto = proto ? strdup(proto) : NULL; > - if (res_host) > - *res_host = strdup(host); > - if (res_port) > - *res_port = port; > - if (res_path) > - *res_path = (path && *path) ? strdup(path) : NULL; > - > - /* Undo the damage we did to the original string */ > - if (port_str) > - *(port_str) = ':'; > - if (path) > - *(path - 1) = '/'; > - if (proto) > - *(host - 3) = ':'; > - return 0; > + int port = default_port; > + > + xmlURIPtr uriInfo = xmlParseURI(url); > + if (uriInfo == NULL || uriInfo->scheme == NULL || uriInfo->server == NULL) > + return -EPROTONOSUPPORT; > + > + *res_proto = strdup(uriInfo->scheme); > + > + if (res_proto != NULL) > + { > + if (!strcasecmp(*res_proto, "https")) > + port = 443; > + else if (!strcasecmp(*res_proto, "http")) > + port = 80; > + else if (!strcasecmp(*res_proto, "socks") || > + !strcasecmp(*res_proto, "socks4") || > + !strcasecmp(*res_proto, "socks5")) > + port = 1080; > + else > + return -EPROTONOSUPPORT; > + } > + > + if (res_port != NULL) > + { > + if (uriInfo->port >= 0) > + port = uriInfo->port; > + *res_port = port; > + } > + > + if (res_host != NULL) > + *res_host = strdup(uriInfo->server); > + > + //user information may contain the password too > + if (res_auth_uname != NULL && res_auth_pwd != NULL && uriInfo->user != NULL) > + { > + char * copy = strdup(uriInfo->user); > + char * separator = strchr(copy, ':'); > + if (separator == NULL) > + *res_auth_uname = copy; > + else > + { > + *res_auth_uname = copy; > + *res_auth_pwd = separator ++; > + *separator = '\0'; > + } > + } > + > + if (res_path != NULL && uriInfo->path != NULL) > + { > + *res_path = strdup(uriInfo->path); > + } > + > + xmlFreeURI(uriInfo); > + > + return 0; //Success > } > > static void clear_cookies(struct openconnect_info *vpninfo) > @@ -758,7 +755,7 @@ static int handle_redirect(struct openconnect_info *vpninfo) > free(vpninfo->urlpath); > vpninfo->urlpath = NULL; > > - ret = internal_parse_url(vpninfo->redirect_url, NULL, &host, &port, &vpninfo->urlpath, 0); > + ret = internal_parse_url(vpninfo->redirect_url, NULL, &host, &port, NULL, NULL, &vpninfo->urlpath, 0); > if (ret) { > vpn_progress(vpninfo, PRG_ERR, > _("Failed to parse redirected URL '%s': %s\n"), > @@ -1533,6 +1530,19 @@ static int process_http_proxy(struct openconnect_info *vpninfo, int ssl_sock) > return -EINVAL; > } > > + //Check for authentication demand from the proxy > + //TODO her, here here! > + if (result == 407) > + { > + //Step 1: extract the token(s) provided and the method(s) supported > + // > + //Step 2: encode the usernmae+password combination in base64 > + // > + //Step 3: Add the tokens and auth info > + // > + //Step 4: Send to proxy and get response > + } > + > if (result != 200) { > vpn_progress(vpninfo, PRG_ERR, > _("Proxy CONNECT request failed: %s\n"), buf); > @@ -1581,7 +1591,7 @@ int openconnect_set_http_proxy(struct openconnect_info *vpninfo, char *proxy) > vpninfo->proxy = NULL; > > ret = internal_parse_url(url, &vpninfo->proxy_type, &vpninfo->proxy, > - &vpninfo->proxy_port, NULL, 80); > + &vpninfo->proxy_port, &vpninfo->proxy_auth_uname, &vpninfo->proxy_auth_pwd, NULL, 80); > if (ret) > goto out; > > diff --git a/library.c b/library.c > index ae91e8d..0a8ee3c 100644 > --- a/library.c > +++ b/library.c > @@ -343,7 +343,7 @@ int openconnect_parse_url(struct openconnect_info *vpninfo, char *url) > vpninfo->urlpath = NULL; > > ret = internal_parse_url(url, &scheme, &vpninfo->hostname, > - &vpninfo->port, &vpninfo->urlpath, 443); > + &vpninfo->port, NULL, NULL, &vpninfo->urlpath, 443); > > if (ret) { > vpn_progress(vpninfo, PRG_ERR, > >
