mirror of
https://github.com/curl/curl.git
synced 2026-08-01 03:28:05 +03:00
lib: avoid reusing unclean connection
When `curl_easy_cleanup()` is invoked while still being part of a multi handle, the code will auto-remove it. But since the connection was detached first, the code in `curl_multi_remove_handle()` that invalidates dirty connections did not bite. Attach the connection *after* the possible remove from a multi handle, so that connection reuse can be prevented. Add test753 to reproduce and verify the fix. This required adding the new debug env var CURL_FTP_PWD_STOP, to have a transfer return from multi_perform() early with a connection that does not show and pending input. Reported-by: Brian Harris Fixes https://github.com/curl/curl/issues/17578 Closes https://github.com/curl/curl/pull/17607
This commit is contained in:
Stefan Eissing
Jay Satiro
parent
17f2a220a9
commit
66d35ee5d4
7 changed files with 257 additions and 13 deletions
|
|
@ -83,6 +83,12 @@ When built with c-ares for name resolving, setting this environment variable
|
|||
to `[IP:port]` makes libcurl use that DNS server instead of the system
|
||||
default. This is used by the curl test suite.
|
||||
|
||||
## `CURL_FTP_PWD_STOP`
|
||||
|
||||
When set, the first transfer - when using ftp: - returns before sending
|
||||
the `PWD` command and stop any further progress. This is used to test
|
||||
an edge case
|
||||
|
||||
## `CURL_GETHOSTNAME`
|
||||
|
||||
Fake the local machine's unqualified hostname for NTLM and SMTP.
|
||||
|
|
|
|||
|
|
@ -765,7 +765,12 @@ static CURLcode ftp_state_user(struct Curl_easy *data,
|
|||
static CURLcode ftp_state_pwd(struct Curl_easy *data,
|
||||
struct ftp_conn *ftpc)
|
||||
{
|
||||
CURLcode result = Curl_pp_sendf(data, &ftpc->pp, "%s", "PWD");
|
||||
CURLcode result;
|
||||
#ifdef DEBUGBUILD
|
||||
if(!data->id && getenv("CURL_FTP_PWD_STOP"))
|
||||
return CURLE_OK;
|
||||
#endif
|
||||
result = Curl_pp_sendf(data, &ftpc->pp, "%s", "PWD");
|
||||
if(!result)
|
||||
ftp_state(data, ftpc, FTP_PWD);
|
||||
|
||||
|
|
|
|||
22
lib/url.c
22
lib/url.c
|
|
@ -234,22 +234,24 @@ CURLcode Curl_close(struct Curl_easy **datap)
|
|||
data = *datap;
|
||||
*datap = NULL;
|
||||
|
||||
/* Detach connection if any is left. This should not be normal, but can be
|
||||
the case for example with CONNECT_ONLY + recv/send (test 556) */
|
||||
Curl_detach_connection(data);
|
||||
if(!data->state.internal) {
|
||||
if(data->multi)
|
||||
/* This handle is still part of a multi handle, take care of this first
|
||||
and detach this handle from there. */
|
||||
curl_multi_remove_handle(data->multi, data);
|
||||
|
||||
if(data->multi_easy) {
|
||||
if(!data->state.internal && data->multi) {
|
||||
/* This handle is still part of a multi handle, take care of this first
|
||||
and detach this handle from there.
|
||||
This detaches the connection. */
|
||||
curl_multi_remove_handle(data->multi, data);
|
||||
}
|
||||
else {
|
||||
/* Detach connection if any is left. This should not be normal, but can be
|
||||
the case for example with CONNECT_ONLY + recv/send (test 556) */
|
||||
Curl_detach_connection(data);
|
||||
if(!data->state.internal && data->multi_easy) {
|
||||
/* when curl_easy_perform() is used, it creates its own multi handle to
|
||||
use and this is the one */
|
||||
curl_multi_cleanup(data->multi_easy);
|
||||
data->multi_easy = NULL;
|
||||
}
|
||||
}
|
||||
DEBUGASSERT(!data->conn || data->state.internal);
|
||||
|
||||
Curl_expire_clear(data); /* shut off any timers left */
|
||||
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ test709 test710 test711 test712 test713 test714 test715 test716 test717 \
|
|||
test718 test719 test720 test721 test722 test723 test724 test725 test726 \
|
||||
test727 test728 test729 test730 test731 test732 test733 test734 test735 \
|
||||
test736 test737 test738 test739 test740 test741 test742 test743 test744 \
|
||||
test745 test746 test747 test748 test749 test750 test751 test752 \
|
||||
test745 test746 test747 test748 test749 test750 test751 test752 test753 \
|
||||
\
|
||||
test780 test781 test782 test783 test784 test785 test786 test787 test788 \
|
||||
test789 test790 test791 \
|
||||
|
|
|
|||
45
tests/data/test753
Normal file
45
tests/data/test753
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
<testcase>
|
||||
<info>
|
||||
<keywords>
|
||||
FTP
|
||||
</keywords>
|
||||
</info>
|
||||
|
||||
# Client-side
|
||||
<client>
|
||||
<features>
|
||||
Debug
|
||||
ftp
|
||||
</features>
|
||||
<server>
|
||||
ftp
|
||||
</server>
|
||||
<tool>
|
||||
lib%TESTNUMBER
|
||||
</tool>
|
||||
<name>
|
||||
cleanup easy without multi_remove_handle
|
||||
</name>
|
||||
<command>
|
||||
ftp://%HOSTIP:%FTPPORT/%TESTNUMBER
|
||||
</command>
|
||||
<setenv>
|
||||
CURL_FTP_PWD_STOP=1
|
||||
</setenv>
|
||||
</client>
|
||||
|
||||
# Verify data after the test has been "shot"
|
||||
<verify>
|
||||
<protocol>
|
||||
USER anonymous
|
||||
PASS ftp@example.com
|
||||
USER anonymous
|
||||
PASS ftp@example.com
|
||||
PWD
|
||||
EPSV
|
||||
TYPE I
|
||||
SIZE 753
|
||||
QUIT
|
||||
</protocol>
|
||||
</verify>
|
||||
</testcase>
|
||||
|
|
@ -59,7 +59,7 @@ TESTFILES = \
|
|||
lib643.c lib650.c lib651.c lib652.c lib653.c lib654.c lib655.c lib658.c \
|
||||
lib659.c lib661.c lib666.c lib667.c lib668.c \
|
||||
lib670.c lib674.c lib676.c lib677.c lib678.c lib694.c lib695.c \
|
||||
lib751.c \
|
||||
lib751.c lib753.c \
|
||||
lib1156.c \
|
||||
lib1301.c \
|
||||
lib1308.c \
|
||||
|
|
|
|||
186
tests/libtest/lib753.c
Normal file
186
tests/libtest/lib753.c
Normal file
|
|
@ -0,0 +1,186 @@
|
|||
/***************************************************************************
|
||||
* _ _ ____ _
|
||||
* Project ___| | | | _ \| |
|
||||
* / __| | | | |_) | |
|
||||
* | (__| |_| | _ <| |___
|
||||
* \___|\___/|_| \_\_____|
|
||||
*
|
||||
* Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
|
||||
*
|
||||
* This software is licensed as described in the file COPYING, which
|
||||
* you should have received as part of this distribution. The terms
|
||||
* are also available at https://curl.se/docs/copyright.html.
|
||||
*
|
||||
* You may opt to use, copy, modify, merge, publish, distribute and/or sell
|
||||
* copies of the Software, and permit persons to whom the Software is
|
||||
* furnished to do so, under the terms of the COPYING file.
|
||||
*
|
||||
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
|
||||
* KIND, either express or implied.
|
||||
*
|
||||
* SPDX-License-Identifier: curl
|
||||
*
|
||||
***************************************************************************/
|
||||
#include "test.h"
|
||||
|
||||
#include "testutil.h"
|
||||
#include "testtrace.h"
|
||||
#include "warnless.h"
|
||||
#include "memdebug.h"
|
||||
|
||||
struct t753_transfer_status {
|
||||
CURL *easy;
|
||||
const char *name;
|
||||
bool pause;
|
||||
bool is_paused;
|
||||
bool seen_welcome;
|
||||
};
|
||||
|
||||
static size_t t753_write_cb(char *ptr, size_t size, size_t nmemb, void *userp)
|
||||
{
|
||||
struct t753_transfer_status *st = userp;
|
||||
size_t len = size * nmemb;
|
||||
(void)ptr;
|
||||
if(st->pause) {
|
||||
curl_mfprintf(stderr, "[%s] write_cb(len=%zu), PAUSE\n", st->name, len);
|
||||
st->is_paused = TRUE;
|
||||
return CURL_READFUNC_PAUSE;
|
||||
}
|
||||
curl_mfprintf(stderr, "[%s] write_cb(len=%zu), CONSUME\n", st->name, len);
|
||||
st->is_paused = FALSE;
|
||||
return len;
|
||||
}
|
||||
|
||||
static size_t t753_hd_cb(char *ptr, size_t size, size_t nmemb, void *userp)
|
||||
{
|
||||
struct t753_transfer_status *st = userp;
|
||||
size_t len = size * nmemb;
|
||||
curl_mfprintf(stderr, "[%s] hd_cb '%.*s'\n", st->name, (int)len, ptr);
|
||||
if(!strcmp("230 Welcome you silly person\r\n", ptr)) {
|
||||
st->seen_welcome = TRUE;
|
||||
st->easy = NULL;
|
||||
}
|
||||
return len;
|
||||
}
|
||||
|
||||
static bool t753_setup(char *URL, const char *name,
|
||||
CURL **peasy,
|
||||
struct t753_transfer_status *st)
|
||||
{
|
||||
CURL *easy = NULL;
|
||||
CURLcode res = CURLE_OK;
|
||||
|
||||
*peasy = NULL;
|
||||
memset(st, 0, sizeof(*st));
|
||||
st->name = name;
|
||||
st->easy = easy;
|
||||
st->pause = TRUE;
|
||||
|
||||
easy_init(easy);
|
||||
|
||||
easy_setopt(easy, CURLOPT_URL, URL);
|
||||
easy_setopt(easy, CURLOPT_WRITEFUNCTION, t753_write_cb);
|
||||
easy_setopt(easy, CURLOPT_WRITEDATA, st);
|
||||
easy_setopt(easy, CURLOPT_HEADERFUNCTION, t753_hd_cb);
|
||||
easy_setopt(easy, CURLOPT_HEADERDATA, st);
|
||||
|
||||
easy_setopt(easy, CURLOPT_NOPROGRESS, 1L);
|
||||
easy_setopt(easy, CURLOPT_DEBUGDATA, &libtest_debug_config);
|
||||
easy_setopt(easy, CURLOPT_DEBUGFUNCTION, libtest_debug_cb);
|
||||
easy_setopt(easy, CURLOPT_VERBOSE, 1L);
|
||||
|
||||
*peasy = easy;
|
||||
return TRUE;
|
||||
|
||||
test_cleanup:
|
||||
if(easy)
|
||||
curl_easy_cleanup(easy);
|
||||
return FALSE;
|
||||
}
|
||||
|
||||
static CURLcode test_lib753(char *URL)
|
||||
{
|
||||
CURL *easy1 = NULL, *easy2 = NULL;
|
||||
CURLM *multi = NULL;
|
||||
struct t753_transfer_status st1, st2;
|
||||
CURLcode res = CURLE_OK;
|
||||
CURLMcode mres;
|
||||
int still_running;
|
||||
|
||||
start_test_timing();
|
||||
|
||||
libtest_debug_config.nohex = 1;
|
||||
libtest_debug_config.tracetime = 1;
|
||||
|
||||
curl_global_init(CURL_GLOBAL_DEFAULT);
|
||||
|
||||
curl_mfprintf(stderr, "init multi\n");
|
||||
multi = curl_multi_init();
|
||||
if(!multi) {
|
||||
res = CURLE_OUT_OF_MEMORY;
|
||||
goto test_cleanup;
|
||||
}
|
||||
|
||||
if(!t753_setup(URL, "EASY1", &easy1, &st1))
|
||||
goto test_cleanup;
|
||||
|
||||
multi_add_handle(multi, easy1);
|
||||
|
||||
multi_perform(multi, &still_running);
|
||||
abort_on_test_timeout();
|
||||
curl_mfprintf(stderr, "multi_perform() -> %d running\n", still_running);
|
||||
|
||||
while(still_running) {
|
||||
int num;
|
||||
|
||||
/* The purpose of this Test:
|
||||
* 1. Violently cleanup EASY1 *without* removing it from the multi
|
||||
* handle first. This MUST discard the connection that EASY1 holds,
|
||||
* as EASY1 is not DONE at this point.
|
||||
* With the env var CURL_FTP_PWD_STOP set, the connection will
|
||||
* have no outstanding data at this point. This would allow
|
||||
* reuse if the connection is not terminated by the cleanup.
|
||||
* 2. Add EASY2 for the same URL and observe in the expected result
|
||||
* that the connection is NOT reused, e.g. all FTP commands
|
||||
* are sent again on the new connection.
|
||||
*/
|
||||
if(easy1 && st1.seen_welcome) {
|
||||
curl_easy_cleanup(easy1);
|
||||
easy1 = NULL;
|
||||
if(!easy2) {
|
||||
if(!t753_setup(URL, "EASY2", &easy2, &st2))
|
||||
goto test_cleanup;
|
||||
st2.pause = FALSE;
|
||||
multi_add_handle(multi, easy2);
|
||||
}
|
||||
}
|
||||
|
||||
mres = curl_multi_wait(multi, NULL, 0, 1, &num);
|
||||
if(mres != CURLM_OK) {
|
||||
curl_mfprintf(stderr, "curl_multi_wait() returned %d\n", mres);
|
||||
res = TEST_ERR_MAJOR_BAD;
|
||||
goto test_cleanup;
|
||||
}
|
||||
|
||||
abort_on_test_timeout();
|
||||
|
||||
multi_perform(multi, &still_running);
|
||||
curl_mfprintf(stderr, "multi_perform() -> %d running\n", still_running);
|
||||
|
||||
abort_on_test_timeout();
|
||||
}
|
||||
|
||||
test_cleanup:
|
||||
|
||||
if(res)
|
||||
curl_mfprintf(stderr, "ERROR: %s\n", curl_easy_strerror(res));
|
||||
|
||||
if(easy1)
|
||||
curl_easy_cleanup(easy1);
|
||||
if(easy2)
|
||||
curl_easy_cleanup(easy2);
|
||||
curl_multi_cleanup(multi);
|
||||
curl_global_cleanup();
|
||||
|
||||
return res;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue