tags 719856 - patch quit Jonathan Nieder wrote:
> How about something like > this patch? Here's a more complete patch against Daniel's "master". It doesn't pass the test suite yet. If this makes sense, I can split it into smaller pieces: 1. use the "goto out" for exception handling in create_conn 2. allocate user, password, and options on the heap instead of the stack 3. handle long usernames and passwords in netrc 4. handle long usernames, passwords, and options from curl_easy_setopt (the title feature!) 5. deal with exceptional cases first and use the "goto out" idiom in parse_url_login 6. handle long usernames and passwords from URL. That would make it easier to find out which change is breaking tests and to review the changes. Thoughts of all kinds welcome, as always. --- lib/netrc.c | 20 ++-- lib/netrc.h | 14 +-- lib/url.c | 264 ++++++++++++++++++++++++++++---------------------- tests/unit/unit1304.c | 53 +++++----- 4 files changed, 196 insertions(+), 155 deletions(-) diff --git a/lib/netrc.c b/lib/netrc.c index 2c5942af..f51fdf34 100644 --- a/lib/netrc.c +++ b/lib/netrc.c @@ -52,13 +52,13 @@ enum host_lookup_state { * @unittest: 1304 */ int Curl_parsenetrc(const char *host, - char *login, - char *password, + char **loginp, + char **passwordp, char *netrcfile) { FILE *file; int retcode=1; - int specific_login = (login[0] != 0); + int specific_login = (**loginp != 0); char *home = NULL; bool home_alloc = FALSE; bool netrc_alloc = FALSE; @@ -109,7 +109,7 @@ int Curl_parsenetrc(const char *host, tok=strtok_r(netrcbuffer, " \t\n", &tok_buf); while(!done && tok) { - if(login[0] && password[0]) { + if(**loginp && **passwordp) { done=TRUE; break; } @@ -138,16 +138,22 @@ int Curl_parsenetrc(const char *host, /* we are now parsing sub-keywords concerning "our" host */ if(state_login) { if(specific_login) { - state_our_login = Curl_raw_equal(login, tok); + state_our_login = Curl_raw_equal(*loginp, tok); } else { - strncpy(login, tok, LOGINSIZE-1); + free(*loginp); + *loginp = strdup(tok); + if(!*loginp) + return -1; /* allocation failed */ } state_login=0; } else if(state_password) { if(state_our_login || !specific_login) { - strncpy(password, tok, PASSWORDSIZE-1); + free(*passwordp); + *passwordp = strdup(tok); + if(!*passwordp) + return -1; /* allocation failed */ } state_password=0; } diff --git a/lib/netrc.h b/lib/netrc.h index 4db764df..6bd9df6f 100644 --- a/lib/netrc.h +++ b/lib/netrc.h @@ -22,19 +22,15 @@ * ***************************************************************************/ -/* Make sure we have room for at least this size: */ -#define LOGINSIZE 64 -#define PASSWORDSIZE 64 - /* returns -1 on failure, 0 if the host is found, 1 is the host isn't found */ int Curl_parsenetrc(const char *host, - char *login, - char *password, + char **loginp, + char **passwordp, char *filename); - /* Assume: password[0]=0, host[0] != 0. - * If login[0] = 0, search for login and password within a machine section + /* Assume: (*passwordp)[0]=0, host[0] != 0. + * If (*loginp)[0] = 0, search for login and password within a machine section * in the netrc. - * If login[0] != 0, search for password within machine and login. + * If (*loginp)[0] != 0, search for password within machine and login. */ #endif /* HEADER_CURL_NETRC_H */ diff --git a/lib/url.c b/lib/url.c index 878f3873..fb56006e 100644 --- a/lib/url.c +++ b/lib/url.c @@ -144,7 +144,7 @@ static void signalPipeClose(struct curl_llist *pipeline, bool pipe_broke); static CURLcode do_init(struct connectdata *conn); static CURLcode parse_url_login(struct SessionHandle *data, struct connectdata *conn, - char *user, char *passwd, char *options); + char **user, char **passwd, char **options); static CURLcode parse_login_details(const char *login, const size_t len, char **userptr, char **passwdptr, char **optionsptr); @@ -3687,7 +3687,8 @@ static CURLcode findprotocol(struct SessionHandle *data, static CURLcode parseurlandfillconn(struct SessionHandle *data, struct connectdata *conn, bool *prot_missing, - char *user, char *passwd, char *options) + char **userp, char **passwdp, + char **optionsp) { char *at; char *fragment; @@ -3931,7 +3932,7 @@ static CURLcode parseurlandfillconn(struct SessionHandle *data, * Parse the login details from the URL and strip them out of * the host name */ - result = parse_url_login(data, conn, user, passwd, options); + result = parse_url_login(data, conn, userp, passwdp, optionsp); if(result != CURLE_OK) return result; @@ -4439,13 +4440,8 @@ static CURLcode parse_proxy_auth(struct SessionHandle *data, */ static CURLcode parse_url_login(struct SessionHandle *data, struct connectdata *conn, - char *user, char *passwd, char *options) + char **user, char **passwd, char **options) { - CURLcode result = CURLE_OK; - char *userp = NULL; - char *passwdp = NULL; - char *optionsp = NULL; - /* At this point, we're hoping all the other special cases have * been taken care of, so conn->host.name is at most * [user[:password][;options]]@]hostname @@ -4456,91 +4452,100 @@ static CURLcode parse_url_login(struct SessionHandle *data, char *ptr = strchr(conn->host.name, '@'); char *login = conn->host.name; - user[0] = 0; /* to make everything well-defined */ - passwd[0] = 0; - options[0] = 0; + DEBUGASSERT(*user == NULL); + DEBUGASSERT(*passwd == NULL); + DEBUGASSERT(*options == NULL); /* We will now try to extract the * possible login information in a string like: * ftp://user:passw...@ftp.my.site:8021/README */ - if(ptr) { - /* There's login information to the left of the @ */ - - conn->host.name = ++ptr; + if(!ptr) { + *user = strdup(""); + *passwd = strdup(""); + *options = strdup(""); + if (!*user || !*passwd || !*options) + return CURLE_OUT_OF_MEMORY; + return CURLE_OK; + } - /* So the hostname is sane. Only bother interpreting the - * results if we could care. It could still be wasted - * work because it might be overtaken by the programmatically - * set user/passwd, but doing that first adds more cases here :-( - */ + /* There's login information to the left of the @ */ - if(data->set.use_netrc != CURL_NETRC_REQUIRED) { - /* We could use the login information in the URL so extract it */ - result = parse_login_details(login, ptr - login - 1, - &userp, &passwdp, &optionsp); - if(!result) { - if(userp) { - char *newname; + conn->host.name = ++ptr; - /* We have a user in the URL */ - conn->bits.userpwd_in_url = TRUE; - conn->bits.user_passwd = TRUE; /* enable user+password */ + /* So the hostname is sane. Only bother interpreting the + * results if we could care. It could still be wasted + * work because it might be overtaken by the programmatically + * set user/passwd, but doing that first adds more cases here :-( + */ + if(data->set.use_netrc == CURL_NETRC_REQUIRED) { + *user = strdup(""); + *passwd = strdup(""); + *options = strdup(""); + if (!*user || !*passwd || !*options) + return CURLE_OUT_OF_MEMORY; + return CURLE_OK; + } - /* Decode the user */ - newname = curl_easy_unescape(data, userp, 0, NULL); - if(!newname) { - Curl_safefree(userp); - Curl_safefree(passwdp); - Curl_safefree(optionsp); - return CURLE_OUT_OF_MEMORY; - } + { + CURLcode result = CURLE_OK; + char *userp = NULL; + char *passwdp = NULL; + char *optionsp = NULL; - if(strlen(newname) < MAX_CURL_USER_LENGTH) - strcpy(user, newname); + /* We could use the login information in the URL so extract it */ + result = parse_login_details(login, ptr - login - 1, + &userp, &passwdp, &optionsp); + if(result) + goto out; - free(newname); - } + if(userp) { + char *newname; - if(passwdp) { - /* We have a password in the URL so decode it */ - char *newpasswd = curl_easy_unescape(data, passwdp, 0, NULL); - if(!newpasswd) { - Curl_safefree(userp); - Curl_safefree(passwdp); - Curl_safefree(optionsp); - return CURLE_OUT_OF_MEMORY; - } + /* We have a user in the URL */ + conn->bits.userpwd_in_url = TRUE; + conn->bits.user_passwd = TRUE; /* enable user+password */ - if(strlen(newpasswd) < MAX_CURL_PASSWORD_LENGTH) - strcpy(passwd, newpasswd); + /* Decode the user */ + *user = curl_easy_unescape(data, userp, 0, NULL); + } + else + /* Allocate an empty string instead of returning NULL as user name */ + *user = strdup(""); - free(newpasswd); - } + if (!*user) { + result = CURLE_OUT_OF_MEMORY; + goto out; + } - if(optionsp) { - /* We have an options list in the URL so decode it */ - char *newoptions = curl_easy_unescape(data, optionsp, 0, NULL); - if(!newoptions) { - Curl_safefree(userp); - Curl_safefree(passwdp); - Curl_safefree(optionsp); - return CURLE_OUT_OF_MEMORY; - } + if(passwdp) + /* We have a password in the URL so decode it */ + *passwd = curl_easy_unescape(data, passwdp, 0, NULL); + else + *passwd = strdup(""); - if(strlen(newoptions) < MAX_CURL_OPTIONS_LENGTH) - strcpy(options, newoptions); + if (!*passwd) { + result = CURLE_OUT_OF_MEMORY; + goto out; + } - free(newoptions); - } - } + if(optionsp) + /* We have an options list in the URL so decode it */ + *options = curl_easy_unescape(data, optionsp, 0, NULL); + else + *options = strdup(""); - Curl_safefree(userp); - Curl_safefree(passwdp); - Curl_safefree(optionsp); + if (!*options) { + result = CURLE_OUT_OF_MEMORY; + goto out; } - } - return result; + out: + + Curl_safefree(userp); + Curl_safefree(passwdp); + Curl_safefree(optionsp); + return result; + } } /* @@ -4793,29 +4798,35 @@ static CURLcode parse_remote_port(struct SessionHandle *data, * Override the login details from the URL with that in the CURLOPT_USERPWD * option or a .netrc file, if applicable. */ -static void override_login(struct SessionHandle *data, - struct connectdata *conn, - char *user, char *passwd, char *options) +static CURLcode override_login(struct SessionHandle *data, + struct connectdata *conn, + char **userp, char **passwdp, char **optionsp) { if(data->set.str[STRING_USERNAME]) { - strncpy(user, data->set.str[STRING_USERNAME], MAX_CURL_USER_LENGTH); - user[MAX_CURL_USER_LENGTH - 1] = '\0'; /* To be on safe side */ + free(*userp); + *userp = strdup(data->set.str[STRING_USERNAME]); + if(!*userp) + return CURLE_OUT_OF_MEMORY; } if(data->set.str[STRING_PASSWORD]) { - strncpy(passwd, data->set.str[STRING_PASSWORD], MAX_CURL_PASSWORD_LENGTH); - passwd[MAX_CURL_PASSWORD_LENGTH - 1] = '\0'; /* To be on safe side */ + free(*passwdp); + *passwdp = strdup(data->set.str[STRING_PASSWORD]); + if(!*passwdp) + return CURLE_OUT_OF_MEMORY; } if(data->set.str[STRING_OPTIONS]) { - strncpy(options, data->set.str[STRING_OPTIONS], MAX_CURL_OPTIONS_LENGTH); - options[MAX_CURL_OPTIONS_LENGTH - 1] = '\0'; /* To be on safe side */ + free(*optionsp); + *optionsp = strdup(data->set.str[STRING_OPTIONS]); + if(!*optionsp) + return CURLE_OUT_OF_MEMORY; } conn->bits.netrc = FALSE; if(data->set.use_netrc != CURL_NETRC_IGNORED) { if(Curl_parsenetrc(conn->host.name, - user, passwd, + userp, passwdp, data->set.str[STRING_NETRC_FILE])) { infof(data, "Couldn't find host %s in the " DOT_CHAR "netrc file; using defaults\n", @@ -5046,9 +5057,9 @@ static CURLcode create_conn(struct SessionHandle *data, struct connectdata *conn; struct connectdata *conn_temp = NULL; size_t urllen; - char user[MAX_CURL_USER_LENGTH]; - char passwd[MAX_CURL_PASSWORD_LENGTH]; - char options[MAX_CURL_OPTIONS_LENGTH]; + char *user = NULL; + char *passwd = NULL; + char *options = NULL; bool reuse; char *proxy = NULL; bool prot_missing = FALSE; @@ -5063,8 +5074,10 @@ static CURLcode create_conn(struct SessionHandle *data, * Check input data *************************************************************/ - if(!data->change.url) - return CURLE_URL_MALFORMAT; + if(!data->change.url) { + result = CURLE_URL_MALFORMAT; + goto out; + } /* First, split up the current URL in parts so that we can use the parts for checking against the already present connections. In order @@ -5072,8 +5085,10 @@ static CURLcode create_conn(struct SessionHandle *data, connection data struct and fill in for comparison purposes. */ conn = allocate_conn(data); - if(!conn) - return CURLE_OUT_OF_MEMORY; + if(!conn) { + result = CURLE_OUT_OF_MEMORY; + goto out; + } /* We must set the return variable as soon as possible, so that our parent can cleanup any possible allocs we may have done before @@ -5103,24 +5118,27 @@ static CURLcode create_conn(struct SessionHandle *data, data->state.path = NULL; data->state.pathbuffer = malloc(urllen+2); - if(NULL == data->state.pathbuffer) - return CURLE_OUT_OF_MEMORY; /* really bad error */ + if(NULL == data->state.pathbuffer) { + result = CURLE_OUT_OF_MEMORY; /* really bad error */ + goto out; + } data->state.path = data->state.pathbuffer; conn->host.rawalloc = malloc(urllen+2); if(NULL == conn->host.rawalloc) { Curl_safefree(data->state.pathbuffer); data->state.path = NULL; - return CURLE_OUT_OF_MEMORY; + result = CURLE_OUT_OF_MEMORY; + goto out; } conn->host.name = conn->host.rawalloc; conn->host.name[0] = 0; - result = parseurlandfillconn(data, conn, &prot_missing, user, passwd, - options); + result = parseurlandfillconn(data, conn, &prot_missing, &user, &passwd, + &options); if(result != CURLE_OK) - return result; + goto out; /************************************************************* * No protocol part in URL was used, add it! @@ -5134,8 +5152,8 @@ static CURLcode create_conn(struct SessionHandle *data, reurl = aprintf("%s://%s", conn->handler->scheme, data->change.url); if(!reurl) { - Curl_safefree(proxy); - return CURLE_OUT_OF_MEMORY; + result = CURLE_OUT_OF_MEMORY; + goto out; } if(data->change.url_alloc) { @@ -5172,7 +5190,7 @@ static CURLcode create_conn(struct SessionHandle *data, if(conn->bits.proxy_user_passwd) { result = parse_proxy_auth(data, conn); if(result != CURLE_OK) - return result; + goto out; } /************************************************************* @@ -5183,7 +5201,8 @@ static CURLcode create_conn(struct SessionHandle *data, /* if global proxy is set, this is it */ if(NULL == proxy) { failf(data, "memory shortage"); - return CURLE_OUT_OF_MEMORY; + result = CURLE_OUT_OF_MEMORY; + goto out; } } @@ -5214,13 +5233,14 @@ static CURLcode create_conn(struct SessionHandle *data, free(proxy); /* parse_proxy copies the proxy string */ if(result) - return result; + goto out; if((conn->proxytype == CURLPROXY_HTTP) || (conn->proxytype == CURLPROXY_HTTP_1_0)) { #ifdef CURL_DISABLE_HTTP /* asking for a HTTP proxy is a bit funny when HTTP is disabled... */ - return CURLE_UNSUPPORTED_PROTOCOL; + result = CURLE_UNSUPPORTED_PROTOCOL; + goto out; #else /* force this connection's protocol to become HTTP if not already compatible - if it isn't tunneling through */ @@ -5257,24 +5277,25 @@ static CURLcode create_conn(struct SessionHandle *data, *************************************************************/ result = parse_remote_port(data, conn); if(result != CURLE_OK) - return result; + goto out; /* Check for overridden login details and set them accordingly so they they are known when protocol->setup_connection is called! */ - override_login(data, conn, user, passwd, options); + result = override_login(data, conn, &user, &passwd, &options); + if(result != CURLE_OK) + goto out; + result = set_login(conn, user, passwd, options); if(result != CURLE_OK) - return result; + goto out; /************************************************************* * Setup internals depending on protocol. Needs to be done after * we figured out what/if proxy to use. *************************************************************/ result = setup_connection_internals(conn); - if(result != CURLE_OK) { - Curl_safefree(proxy); - return result; - } + if(result != CURLE_OK) + goto out; conn->recv[FIRSTSOCKET] = Curl_recv_plain; conn->send[FIRSTSOCKET] = Curl_send_plain; @@ -5307,7 +5328,7 @@ static CURLcode create_conn(struct SessionHandle *data, DEBUGASSERT(conn->handler->done); /* we ignore the return code for the protocol-specific DONE */ (void)conn->handler->done(conn, result, FALSE); - return result; + goto out; } Curl_setup_transfer(conn, -1, -1, FALSE, NULL, /* no download */ @@ -5317,7 +5338,7 @@ static CURLcode create_conn(struct SessionHandle *data, /* since we skip do_init() */ do_init(conn); - return result; + goto out; } #endif @@ -5342,8 +5363,10 @@ static CURLcode create_conn(struct SessionHandle *data, data->set.ssl.password = data->set.str[STRING_TLSAUTH_PASSWORD]; #endif - if(!Curl_clone_ssl_config(&data->set.ssl, &conn->ssl_config)) - return CURLE_OUT_OF_MEMORY; + if(!Curl_clone_ssl_config(&data->set.ssl, &conn->ssl_config)) { + result = CURLE_OUT_OF_MEMORY; + goto out; + } /************************************************************* * Check the current list of connections to see if we can @@ -5446,7 +5469,8 @@ static CURLcode create_conn(struct SessionHandle *data, conn_free(conn); *in_connect = NULL; - return CURLE_NO_CONNECTION_AVAILABLE; + result = CURLE_NO_CONNECTION_AVAILABLE; + goto out; } else { /* @@ -5468,7 +5492,7 @@ static CURLcode create_conn(struct SessionHandle *data, */ result = setup_range(data); if(result) - return result; + goto out; /* Continue connectdata initialization here. */ @@ -5486,6 +5510,12 @@ static CURLcode create_conn(struct SessionHandle *data, *************************************************************/ result = resolve_server(data, conn, async); + out: + + Curl_safefree(proxy); + Curl_safefree(options); + Curl_safefree(passwd); + Curl_safefree(user); return result; } diff --git a/tests/unit/unit1304.c b/tests/unit/unit1304.c index 8ddd8cae..1a4589f8 100644 --- a/tests/unit/unit1304.c +++ b/tests/unit/unit1304.c @@ -23,14 +23,14 @@ #include "netrc.h" -static char login[LOGINSIZE]; -static char password[PASSWORDSIZE]; +static char *login; +static char *password; static char filename[64]; static CURLcode unit_setup(void) { - password[0] = 0; - login[0] = 0; + password = strdup(""); + login = strdup(""); return CURLE_OK; } @@ -47,7 +47,7 @@ UNITTEST_START /* * Test a non existent host in our netrc file. */ - result = Curl_parsenetrc("test.example.com", login, password, filename); + result = Curl_parsenetrc("test.example.com", &login, &password, filename); fail_unless(result == 1, "Host not found should return 1"); fail_unless(password[0] == 0, "password should not have been changed"); fail_unless(login[0] == 0, "login should not have been changed"); @@ -55,8 +55,9 @@ UNITTEST_START /* * Test a non existent login in our netrc file. */ - memcpy(login, "me", 2); - result = Curl_parsenetrc("example.com", login, password, filename); + free(login); + login = strdup("me"); + result = Curl_parsenetrc("example.com", &login, &password, filename); fail_unless(result == 0, "Host should be found"); fail_unless(password[0] == 0, "password should not have been changed"); fail_unless(strncmp(login, "me", 2) == 0, "login should not have been changed"); @@ -64,8 +65,9 @@ UNITTEST_START /* * Test a non existent login and host in our netrc file. */ - memcpy(login, "me", 2); - result = Curl_parsenetrc("test.example.com", login, password, filename); + free(login); + login = strdup("me"); + result = Curl_parsenetrc("test.example.com", &login, &password, filename); fail_unless(result == 1, "Host should be found"); fail_unless(password[0] == 0, "password should not have been changed"); fail_unless(strncmp(login, "me", 2) == 0, "login should not have been changed"); @@ -74,8 +76,9 @@ UNITTEST_START * Test a non existent login (substring of an existing one) in our * netrc file. */ - memcpy(login, "admi", 4); - result = Curl_parsenetrc("example.com", login, password, filename); + free(login); + login = strdup("admi"); + result = Curl_parsenetrc("example.com", &login, &password, filename); fail_unless(result == 0, "Host should be found"); fail_unless(password[0] == 0, "password should not have been changed"); fail_unless(strncmp(login, "admi", 4) == 0, "login should not have been changed"); @@ -84,8 +87,9 @@ UNITTEST_START * Test a non existent login (superstring of an existing one) * in our netrc file. */ - memcpy(login, "adminn", 6); - result = Curl_parsenetrc("example.com", login, password, filename); + free(login); + login = strdup("adminn"); + result = Curl_parsenetrc("example.com", &login, &password, filename); fail_unless(result == 0, "Host should be found"); fail_unless(password[0] == 0, "password should not have been changed"); fail_unless(strncmp(login, "adminn", 6) == 0, "login should not have been changed"); @@ -94,8 +98,9 @@ UNITTEST_START * Test for the first existing host in our netrc file * with login[0] = 0. */ - login[0] = 0; - result = Curl_parsenetrc("example.com", login, password, filename); + free(login) + login = strdup(""); + result = Curl_parsenetrc("example.com", &login, &password, filename); fail_unless(result == 0, "Host should have been found"); fail_unless(strncmp(password, "passwd", 6) == 0, "password should be 'passwd'"); @@ -105,8 +110,9 @@ UNITTEST_START * Test for the first existing host in our netrc file * with login[0] != 0. */ - password[0] = 0; - result = Curl_parsenetrc("example.com", login, password, filename); + free(password); + password = strdup(""); + result = Curl_parsenetrc("example.com", &login, &password, filename); fail_unless(result == 0, "Host should have been found"); fail_unless(strncmp(password, "passwd", 6) == 0, "password should be 'passwd'"); @@ -116,9 +122,11 @@ UNITTEST_START * Test for the second existing host in our netrc file * with login[0] = 0. */ - password[0] = 0; - login[0] = 0; - result = Curl_parsenetrc("curl.example.com", login, password, filename); + free(password); + password = strdup(""); + free(login); + login = strdup(""); + result = Curl_parsenetrc("curl.example.com", &login, &password, filename); fail_unless(result == 0, "Host should have been found"); fail_unless(strncmp(password, "none", 4) == 0, "password should be 'none'"); @@ -128,8 +136,9 @@ UNITTEST_START * Test for the second existing host in our netrc file * with login[0] != 0. */ - password[0] = 0; - result = Curl_parsenetrc("curl.example.com", login, password, filename); + free(password); + password = strdup(""); + result = Curl_parsenetrc("curl.example.com", &login, &password, filename); fail_unless(result == 0, "Host should have been found"); fail_unless(strncmp(password, "none", 4) == 0, "password should be 'none'"); -- 1.8.4.rc2 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org