From 0dd1219668de43b675a0f3f1c7521b5f823c6895 Mon Sep 17 00:00:00 2001 From: Dan Fandrich Date: Sat, 24 Mar 2007 02:15:20 +0000 Subject: [PATCH] Fixed a memory leak when specifying a proxy with a file: URL and added test case 288 to verify it. --- CHANGES | 2 + lib/url.c | 129 ++++++++++++++++++++++------------------- tests/data/Makefile.am | 2 +- tests/data/test288 | 42 ++++++++++++++ 4 files changed, 114 insertions(+), 61 deletions(-) create mode 100644 tests/data/test288 diff --git a/CHANGES b/CHANGES index 950ea5b86..38e9a6ec8 100644 --- a/CHANGES +++ b/CHANGES @@ -16,6 +16,8 @@ Dan F (23 March 2007) - Added test harness infrastructure to support scp/sftp tests, using OpenSSH as the server. +- Fixed a memory leak when specifying a proxy with a file: URL. + Yang Tse (20 March 2007) - Fixed: When a signal was caught awaiting for an event using Curl_select() or Curl_poll() with a non-zero timeout both functions would restart the diff --git a/lib/url.c b/lib/url.c index 7ff268e0a..5a882a3b4 100644 --- a/lib/url.c +++ b/lib/url.c @@ -2767,8 +2767,7 @@ static CURLcode CreateConnection(struct SessionHandle *data, char passwd[MAX_CURL_PASSWORD_LENGTH]; int rc; bool reuse; - char *proxy; - bool proxy_alloc = FALSE; + char *proxy = NULL; #ifndef USE_ARES #ifdef SIGALRM @@ -2821,7 +2820,7 @@ static CURLcode CreateConnection(struct SessionHandle *data, conn->bits.proxy = (bool)(data->set.proxy && *data->set.proxy); conn->bits.httpproxy = (bool)(conn->bits.proxy && (conn->proxytype == CURLPROXY_HTTP)); - proxy = data->set.proxy; /* if global proxy is set, this is it */ + /* Default protocol-independent behavior doesn't support persistent connections, so we set this to force-close. Protocols that support @@ -2917,6 +2916,15 @@ static CURLcode CreateConnection(struct SessionHandle *data, return CURLE_OUT_OF_MEMORY; } + if (data->set.proxy) { + proxy = strdup(data->set.proxy); /* if global proxy is set, this is it */ + if(NULL == proxy) { + failf(data, "memory shortage"); + return CURLE_OUT_OF_MEMORY; + } + } + /* proxy must be freed later unless NULL */ + #ifndef CURL_DISABLE_HTTP /************************************************************* * Detect what (if any) proxy to use. Remember that this selects a host @@ -3018,8 +3026,6 @@ static CURLcode CreateConnection(struct SessionHandle *data, if(proxy && *proxy) { long bits = conn->protocol & (PROT_HTTPS|PROT_SSL|PROT_MISSING); - proxy_alloc=TRUE; /* this needs to be freed later */ - if(conn->proxytype == CURLPROXY_HTTP) { /* force this connection's protocol to become HTTP */ conn->protocol = PROT_HTTP | bits; @@ -3044,35 +3050,16 @@ static CURLcode CreateConnection(struct SessionHandle *data, reurl = aprintf("%s://%s", conn->protostr, data->change.url); - if(!reurl) + if(!reurl) { + Curl_safefree(proxy); return CURLE_OUT_OF_MEMORY; + } data->change.url = reurl; data->change.url_alloc = TRUE; /* free this later */ conn->protocol &= ~PROT_MISSING; /* switch that one off again */ } -#ifndef CURL_DISABLE_HTTP - /************************************************************ - * RESUME on a HTTP page is a tricky business. First, let's just check that - * 'range' isn't used, then set the range parameter and leave the resume as - * it is to inform about this situation for later use. We will then - * "attempt" to resume, and if we're talking to a HTTP/1.1 (or later) - * server, we will get the document resumed. If we talk to a HTTP/1.0 - * server, we just fail since we can't rewind the file writing from within - * this function. - ***********************************************************/ - if(data->reqdata.resume_from) { - if(!data->reqdata.use_range) { - /* if it already was in use, we just skip this */ - data->reqdata.range = aprintf("%" FORMAT_OFF_T "-", data->reqdata.resume_from); - if(!data->reqdata.range) - return CURLE_OUT_OF_MEMORY; - data->reqdata.rangestringalloc = TRUE; /* mark as allocated */ - data->reqdata.use_range = 1; /* switch on range usage */ - } - } -#endif /************************************************************* * Setup internals depending on protocol *************************************************************/ @@ -3206,6 +3193,7 @@ static CURLcode CreateConnection(struct SessionHandle *data, #else failf(data, LIBCURL_NAME " was built with TELNET disabled!"); + return CURLE_UNSUPPORTED_PROTOCOL; #endif } else if (strequal(conn->protostr, "DICT")) { @@ -3219,6 +3207,7 @@ static CURLcode CreateConnection(struct SessionHandle *data, #else failf(data, LIBCURL_NAME " was built with DICT disabled!"); + return CURLE_UNSUPPORTED_PROTOCOL; #endif } else if (strequal(conn->protostr, "LDAP")) { @@ -3232,6 +3221,7 @@ static CURLcode CreateConnection(struct SessionHandle *data, #else failf(data, LIBCURL_NAME " was built with LDAP disabled!"); + return CURLE_UNSUPPORTED_PROTOCOL; #endif } else if (strequal(conn->protostr, "FILE")) { @@ -3240,25 +3230,10 @@ static CURLcode CreateConnection(struct SessionHandle *data, conn->curl_do = Curl_file; conn->curl_done = Curl_file_done; - - /* anyway, this is supposed to be the connect function so we better - at least check that the file is present here! */ - result = Curl_file_connect(conn); - - /* Setup a "faked" transfer that'll do nothing */ - if(CURLE_OK == result) { - conn->data = data; - conn->bits.tcpconnect = TRUE; /* we are "connected */ - ConnectionStore(data, conn); - - result = Curl_setup_transfer(conn, -1, -1, FALSE, NULL, /* no download */ - -1, NULL); /* no upload */ - } - - return result; #else failf(data, LIBCURL_NAME " was built with FILE disabled!"); + return CURLE_UNSUPPORTED_PROTOCOL; #endif } else if (strequal(conn->protostr, "TFTP")) { @@ -3297,6 +3272,7 @@ static CURLcode CreateConnection(struct SessionHandle *data, #else failf(data, LIBCURL_NAME " was built with TFTP disabled!"); + return CURLE_UNSUPPORTED_PROTOCOL; #endif } else if (strequal(conn->protostr, "SCP")) { @@ -3344,21 +3320,11 @@ else { char *prox_portno; char *endofprot; - /* We need to make a duplicate of the proxy so that we can modify the - string safely. If 'proxy_alloc' is TRUE, the string is already - allocated and we can treat it as duplicated. */ - char *proxydup=proxy_alloc?proxy:strdup(proxy); - /* We use 'proxyptr' to point to the proxy name from now on... */ - char *proxyptr=proxydup; + char *proxyptr=proxy; char *portptr; char *atsign; - if(NULL == proxydup) { - failf(data, "memory shortage"); - return CURLE_OUT_OF_MEMORY; - } - /* We do the proxy host string parsing here. We want the host name and the * port name. Accept a protocol:// prefix, even though it should just be * ignored. @@ -3404,15 +3370,15 @@ else { atsign = strdup(atsign+1); /* the right side of the @-letter */ if(atsign) { - free(proxydup); /* free the former proxy string */ - proxydup = proxyptr = atsign; /* now use this instead */ + free(proxy); /* free the former proxy string */ + proxy = proxyptr = atsign; /* now use this instead */ } else res = CURLE_OUT_OF_MEMORY; } if(res) { - free(proxydup); /* free the allocated proxy string */ + free(proxy); /* free the allocated proxy string */ return res; } } @@ -3455,12 +3421,33 @@ else { conn->proxy.rawalloc = strdup(proxyptr); conn->proxy.name = conn->proxy.rawalloc; - free(proxydup); /* free the duplicate pointer and not the modified */ - proxy = NULL; /* this may have just been freed */ + free(proxy); + proxy = NULL; if(!conn->proxy.rawalloc) return CURLE_OUT_OF_MEMORY; } + /*********************************************************************** + * file: is a special case in that it doesn't need a network connection + ***********************************************************************/ + if (strequal(conn->protostr, "FILE")) { + /* anyway, this is supposed to be the connect function so we better + at least check that the file is present here! */ + result = Curl_file_connect(conn); + + /* Setup a "faked" transfer that'll do nothing */ + if(CURLE_OK == result) { + conn->data = data; + conn->bits.tcpconnect = TRUE; /* we are "connected */ + ConnectionStore(data, conn); + + result = Curl_setup_transfer(conn, -1, -1, FALSE, NULL, /* no download */ + -1, NULL); /* no upload */ + } + + return result; + } + /************************************************************* * If the protocol is using SSL and HTTP proxy is used, we set * the tunnel_proxy bit. @@ -3678,6 +3665,28 @@ else { if(!conn->user || !conn->passwd) return CURLE_OUT_OF_MEMORY; +#ifndef CURL_DISABLE_HTTP + /************************************************************ + * RESUME on a HTTP page is a tricky business. First, let's just check that + * 'range' isn't used, then set the range parameter and leave the resume as + * it is to inform about this situation for later use. We will then + * "attempt" to resume, and if we're talking to a HTTP/1.1 (or later) + * server, we will get the document resumed. If we talk to a HTTP/1.0 + * server, we just fail since we can't rewind the file writing from within + * this function. + ***********************************************************/ + if(data->reqdata.resume_from) { + if(!data->reqdata.use_range) { + /* if it already was in use, we just skip this */ + data->reqdata.range = aprintf("%" FORMAT_OFF_T "-", data->reqdata.resume_from); + if(!data->reqdata.range) + return CURLE_OUT_OF_MEMORY; + data->reqdata.rangestringalloc = TRUE; /* mark as allocated */ + data->reqdata.use_range = 1; /* switch on range usage */ + } + } +#endif + /************************************************************* * Check the current list of connections to see if we can * re-use an already existing one or if we have to create a @@ -3797,7 +3806,7 @@ else { infof(data, "Re-using existing connection! (#%ld) with host %s\n", conn->connectindex, - proxy?conn->proxy.dispname:conn->host.dispname); + conn->proxy.name?conn->proxy.dispname:conn->host.dispname); } else { /* diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 440588511..f5d3039f9 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -37,4 +37,4 @@ EXTRA_DIST = test1 test108 test117 test127 test20 test27 test34 test46 \ test274 test275 test524 test525 test276 test277 test526 test527 test528 \ test530 DISABLED test278 test279 test531 test280 test529 test532 test533 \ test534 test535 test281 test537 test282 test283 test284 test538 test285 \ - test286 test307 test308 test287 test400 + test286 test307 test308 test287 test400 test288 diff --git a/tests/data/test288 b/tests/data/test288 new file mode 100644 index 000000000..da691451a --- /dev/null +++ b/tests/data/test288 @@ -0,0 +1,42 @@ + +# Server-side + + +foo + bar +bar + foo +moo + + + +# Client-side + + +none + + +file + + +file:// with (unsupported) proxy, authentication and range + + +all_proxy=http://fake:user@%HOSTIP:%HTTPPORT/ + + +file://localhost/%PWD/log/test288.txt + + +foo + bar +bar + foo +moo + + + +# Verify data after the test has been "shot" + + +