From 51c0ebcff2140c38ff389b4fcfb8216f5e9d198c Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Thu, 6 May 2021 13:04:03 +0200 Subject: [PATCH] http: deal with partial CONNECT sends Also added 'CURL_SMALLSENDS' to make Curl_write() send short packets, which helped verifying this even more. Add test 363 to verify. Reported-by: ustcqidi on github Fixes #6950 Closes #7024 --- lib/http.h | 3 +- lib/http_proxy.c | 109 +++++++++++++++++++++++++++++----------- lib/http_proxy.h | 24 +++++++++ lib/multi.c | 8 ++- lib/sendf.c | 12 +++++ lib/urldata.h | 19 +------ tests/data/Makefile.inc | 2 +- tests/data/test363 | 90 +++++++++++++++++++++++++++++++++ 8 files changed, 213 insertions(+), 54 deletions(-) create mode 100644 tests/data/test363 diff --git a/lib/http.h b/lib/http.h index 851e0a382..2a3834ae1 100644 --- a/lib/http.h +++ b/lib/http.h @@ -184,8 +184,7 @@ struct HTTP { enum { HTTPSEND_NADA, /* init */ HTTPSEND_REQUEST, /* sending a request */ - HTTPSEND_BODY, /* sending body */ - HTTPSEND_LAST /* never use this */ + HTTPSEND_BODY /* sending body */ } sending; #ifndef CURL_DISABLE_HTTP diff --git a/lib/http_proxy.c b/lib/http_proxy.c index f403ffc0e..a3a62c1ca 100644 --- a/lib/http_proxy.c +++ b/lib/http_proxy.c @@ -39,6 +39,8 @@ #include "connect.h" #include "curlx.h" #include "vtls/vtls.h" +#include "transfer.h" +#include "multiif.h" /* The last 3 #include files should be in this order */ #include "curl_printf.h" @@ -88,29 +90,12 @@ CURLcode Curl_proxy_connect(struct Curl_easy *data, int sockindex) if(conn->bits.tunnel_proxy && conn->bits.httpproxy) { #ifndef CURL_DISABLE_PROXY /* for [protocol] tunneled through HTTP proxy */ - struct HTTP http_proxy; - void *prot_save; const char *hostname; int remote_port; CURLcode result; - /* BLOCKING */ /* We want "seamless" operations through HTTP proxy tunnel */ - /* Curl_proxyCONNECT is based on a pointer to a struct HTTP at the - * member conn->proto.http; we want [protocol] through HTTP and we have - * to change the member temporarily for connecting to the HTTP - * proxy. After Curl_proxyCONNECT we have to set back the member to the - * original pointer - * - * This function might be called several times in the multi interface case - * if the proxy's CONNECT response is not instant. - */ - prot_save = data->req.p.http; - memset(&http_proxy, 0, sizeof(http_proxy)); - data->req.p.http = &http_proxy; - connkeep(conn, "HTTP proxy CONNECT"); - /* for the secondary socket (FTP), use the "connect to host" * but ignore the "connect to port" (use the secondary port) */ @@ -128,8 +113,8 @@ CURLcode Curl_proxy_connect(struct Curl_easy *data, int sockindex) remote_port = conn->conn_to_port; else remote_port = conn->remote_port; + result = Curl_proxyCONNECT(data, sockindex, hostname, remote_port); - data->req.p.http = prot_save; if(CURLE_OK != result) return result; Curl_safefree(data->state.aptr.proxyuserpwd); @@ -153,18 +138,53 @@ bool Curl_connect_ongoing(struct connectdata *conn) (conn->connect_state->tunnel_state != TUNNEL_COMPLETE); } +/* when we've sent a CONNECT to a proxy, we should rather either wait for the + socket to become readable to be able to get the response headers or if + we're still sending the request, wait for write. */ +int Curl_connect_getsock(struct connectdata *conn) +{ + struct HTTP *http; + DEBUGASSERT(conn); + DEBUGASSERT(conn->connect_state); + http = &conn->connect_state->http_proxy; + + if(http->sending) + return GETSOCK_WRITESOCK(0); + + return GETSOCK_READSOCK(0); +} + static CURLcode connect_init(struct Curl_easy *data, bool reinit) { struct http_connect_state *s; struct connectdata *conn = data->conn; if(!reinit) { + CURLcode result; DEBUGASSERT(!conn->connect_state); + /* we might need the upload buffer for streaming a partial request */ + result = Curl_get_upload_buffer(data); + if(result) + return result; + s = calloc(1, sizeof(struct http_connect_state)); if(!s) return CURLE_OUT_OF_MEMORY; infof(data, "allocate connect buffer!\n"); conn->connect_state = s; Curl_dyn_init(&s->rcvbuf, DYN_PROXY_CONNECT_HEADERS); + + /* Curl_proxyCONNECT is based on a pointer to a struct HTTP at the + * member conn->proto.http; we want [protocol] through HTTP and we have + * to change the member temporarily for connecting to the HTTP + * proxy. After Curl_proxyCONNECT we have to set back the member to the + * original pointer + * + * This function might be called several times in the multi interface case + * if the proxy's CONNECT response is not instant. + */ + s->prot_save = data->req.p.http; + data->req.p.http = &s->http_proxy; + connkeep(conn, "HTTP proxy CONNECT"); } else { DEBUGASSERT(conn->connect_state); @@ -184,6 +204,10 @@ static void connect_done(struct Curl_easy *data) struct http_connect_state *s = conn->connect_state; s->tunnel_state = TUNNEL_COMPLETE; Curl_dyn_free(&s->rcvbuf); + Curl_dyn_free(&s->req); + + /* retore the protocol pointer */ + data->req.p.http = s->prot_save; infof(data, "CONNECT phase completed!\n"); } @@ -231,6 +255,7 @@ static CURLcode CONNECT(struct Curl_easy *data, struct connectdata *conn = data->conn; curl_socket_t tunnelsocket = conn->sock[sockindex]; struct http_connect_state *s = conn->connect_state; + struct HTTP *http = data->req.p.http; char *linep; size_t perline; @@ -246,7 +271,7 @@ static CURLcode CONNECT(struct Curl_easy *data, timediff_t check; if(TUNNEL_INIT == s->tunnel_state) { /* BEGIN CONNECT PHASE */ - struct dynbuf req; + struct dynbuf *req = &s->req; char *hostheader = NULL; char *host = NULL; @@ -259,8 +284,8 @@ static CURLcode CONNECT(struct Curl_easy *data, free(data->req.newurl); data->req.newurl = NULL; - /* initialize a dynamic send-buffer */ - Curl_dyn_init(&req, DYN_HTTP_REQUEST); + /* initialize send-buffer */ + Curl_dyn_init(req, DYN_HTTP_REQUEST); result = CONNECT_host(data, conn, hostname, remote_port, &hostheader, &host); @@ -285,7 +310,7 @@ static CURLcode CONNECT(struct Curl_easy *data, useragent = data->state.aptr.uagent; result = - Curl_dyn_addf(&req, + Curl_dyn_addf(req, "CONNECT %s HTTP/%s\r\n" "%s" /* Host: */ "%s" /* Proxy-Authorization */ @@ -300,16 +325,15 @@ static CURLcode CONNECT(struct Curl_easy *data, proxyconn); if(!result) - result = Curl_add_custom_headers(data, TRUE, &req); + result = Curl_add_custom_headers(data, TRUE, req); if(!result) /* CRLF terminate the request */ - result = Curl_dyn_add(&req, "\r\n"); + result = Curl_dyn_add(req, "\r\n"); if(!result) { /* Send the connect request to the proxy */ - /* BLOCKING */ - result = Curl_buffer_send(&req, data, &data->info.request_size, 0, + result = Curl_buffer_send(req, data, &data->info.request_size, 0, sockindex); } if(result) @@ -317,7 +341,6 @@ static CURLcode CONNECT(struct Curl_easy *data, } free(host); free(hostheader); - Curl_dyn_free(&req); if(result) return result; @@ -330,12 +353,42 @@ static CURLcode CONNECT(struct Curl_easy *data, return CURLE_OPERATION_TIMEDOUT; } - if(!Curl_conn_data_pending(conn, sockindex)) + if(!Curl_conn_data_pending(conn, sockindex) && !http->sending) /* return so we'll be called again polling-style */ return CURLE_OK; /* at this point, the tunnel_connecting phase is over. */ + if(http->sending == HTTPSEND_REQUEST) { + if(!s->nsend) { + size_t fillcount; + k->upload_fromhere = data->state.ulbuf; + result = Curl_fillreadbuffer(data, data->set.upload_buffer_size, + &fillcount); + if(result) + return result; + s->nsend = fillcount; + } + if(s->nsend) { + ssize_t bytes_written; + /* write to socket (send away data) */ + result = Curl_write(data, + conn->writesockfd, /* socket to send to */ + k->upload_fromhere, /* buffer pointer */ + s->nsend, /* buffer size */ + &bytes_written); /* actually sent */ + + if(!result) + /* send to debug callback! */ + result = Curl_debug(data, CURLINFO_HEADER_OUT, + k->upload_fromhere, bytes_written); + + s->nsend -= bytes_written; + k->upload_fromhere += bytes_written; + return result; + } + /* if nothing left to send, continue */ + } { /* READING RESPONSE PHASE */ int error = SELECT_OK; diff --git a/lib/http_proxy.h b/lib/http_proxy.h index a78db0d04..f5a4cb07c 100644 --- a/lib/http_proxy.h +++ b/lib/http_proxy.h @@ -38,15 +38,39 @@ CURLcode Curl_proxy_connect(struct Curl_easy *data, int sockindex); bool Curl_connect_complete(struct connectdata *conn); bool Curl_connect_ongoing(struct connectdata *conn); +int Curl_connect_getsock(struct connectdata *conn); #else #define Curl_proxyCONNECT(x,y,z,w) CURLE_NOT_BUILT_IN #define Curl_proxy_connect(x,y) CURLE_OK #define Curl_connect_complete(x) CURLE_OK #define Curl_connect_ongoing(x) FALSE +#define Curl_connect_getsock(x) 0 #endif void Curl_connect_free(struct Curl_easy *data); void Curl_connect_done(struct Curl_easy *data); +/* struct for HTTP CONNECT state data */ +struct http_connect_state { + struct HTTP http_proxy; + struct HTTP *prot_save; + struct dynbuf rcvbuf; + struct dynbuf req; + size_t nsend; + enum keeponval { + KEEPON_DONE, + KEEPON_CONNECT, + KEEPON_IGNORE + } keepon; + curl_off_t cl; /* size of content to read and ignore */ + enum { + TUNNEL_INIT, /* init/default/no tunnel state */ + TUNNEL_CONNECT, /* CONNECT has been sent off */ + TUNNEL_COMPLETE /* CONNECT response received completely */ + } tunnel_state; + BIT(chunked_encoding); + BIT(close_connection); +}; + #endif /* HEADER_CURL_HTTP_PROXY_H */ diff --git a/lib/multi.c b/lib/multi.c index 205045461..f7b1da9ab 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -936,10 +936,8 @@ static int waitproxyconnect_getsock(struct connectdata *conn, { sock[0] = conn->sock[FIRSTSOCKET]; - /* when we've sent a CONNECT to a proxy, we should rather wait for the - socket to become readable to be able to get the response headers */ if(conn->connect_state) - return GETSOCK_READSOCK(0); + return Curl_connect_getsock(conn); return GETSOCK_WRITESOCK(0); } @@ -1111,11 +1109,11 @@ static CURLMcode multi_wait(struct Curl_multi *multi, for(i = 0; i< MAX_SOCKSPEREASYHANDLE; i++) { curl_socket_t s = CURL_SOCKET_BAD; - if(bitmap & GETSOCK_READSOCK(i)) { + if((bitmap & GETSOCK_READSOCK(i)) && VALID_SOCK((sockbunch[i]))) { ++nfds; s = sockbunch[i]; } - if(bitmap & GETSOCK_WRITESOCK(i)) { + if((bitmap & GETSOCK_WRITESOCK(i)) && VALID_SOCK((sockbunch[i]))) { ++nfds; s = sockbunch[i]; } diff --git a/lib/sendf.c b/lib/sendf.c index fa3c5a35d..e41bb805f 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -309,6 +309,18 @@ CURLcode Curl_write(struct Curl_easy *data, conn = data->conn; num = (sockfd == conn->sock[SECONDARYSOCKET]); +#ifdef CURLDEBUG + { + /* Allow debug builds to override this logic to force short sends + */ + char *p = getenv("CURL_SMALLSENDS"); + if(p) { + size_t altsize = (size_t)strtoul(p, NULL, 10); + if(altsize) + len = CURLMIN(len, altsize); + } + } +#endif bytes_written = conn->send[num](data, num, mem, len, &result); *written = bytes_written; diff --git a/lib/urldata.h b/lib/urldata.h index 421a86b62..707e91f8e 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -858,25 +858,8 @@ struct proxy_info { char *passwd; /* proxy password string, allocated */ }; -/* struct for HTTP CONNECT state data */ -struct http_connect_state { - struct dynbuf rcvbuf; - enum keeponval { - KEEPON_DONE, - KEEPON_CONNECT, - KEEPON_IGNORE - } keepon; - curl_off_t cl; /* size of content to read and ignore */ - enum { - TUNNEL_INIT, /* init/default/no tunnel state */ - TUNNEL_CONNECT, /* CONNECT has been sent off */ - TUNNEL_COMPLETE /* CONNECT response received completely */ - } tunnel_state; - BIT(chunked_encoding); - BIT(close_connection); -}; - struct ldapconninfo; +struct http_connect_state; /* for the (SOCKS) connect state machine */ enum connect_t { diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index f61527089..b0fef53c8 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -60,7 +60,7 @@ test325 test326 test327 test328 test329 test330 test331 test332 test333 \ test334 test335 test336 test337 test338 test339 test340 test341 test342 \ test343 test344 test345 test346 test347 test348 test349 test350 test351 \ test352 test353 test354 test355 test356 test357 test358 test359 test360 \ -test361 test362 \ +test361 test362 test363 \ \ test393 test394 test395 test396 test397 \ \ diff --git a/tests/data/test363 b/tests/data/test363 new file mode 100644 index 000000000..55ef666e2 --- /dev/null +++ b/tests/data/test363 @@ -0,0 +1,90 @@ + + + +HTTP +HTTP POST +HTTP CONNECT +proxytunnel + + + +# +# Server-side + + +HTTP/1.1 200 OK +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Content-Type: text/html +Funny-head: yesyes +Content-Length: 9 + +contents + + +HTTP/1.1 200 Mighty fine indeed + + + +HTTP/1.1 200 Mighty fine indeed + +HTTP/1.1 200 OK +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Content-Type: text/html +Funny-head: yesyes +Content-Length: 9 + +contents + + + +# +# Client-side + + +debug + + +http +http-proxy + + +CONNECT with short sends + + +# make the first send cut off after this amount of data +CURL_SMALLREQSEND=48 +# make repeated sends small too +CURL_SMALLSENDS=40 + + +http://test.%TESTNUMBER:%HTTPPORT/we/want/that/page/%TESTNUMBER -p -x %HOSTIP:%PROXYPORT -d "datatopost=ohthatsfunyesyes" + + +proxy + + + +# +# Verify data after the test has been "shot" + + +CONNECT test.%TESTNUMBER:%HTTPPORT HTTP/1.1 +Host: test.%TESTNUMBER:%HTTPPORT +User-Agent: curl/%VERSION +Proxy-Connection: Keep-Alive + + + +POST /we/want/that/page/%TESTNUMBER HTTP/1.1 +Host: test.%TESTNUMBER:%HTTPPORT +User-Agent: curl/%VERSION +Accept: */* +Content-Length: 27 +Content-Type: application/x-www-form-urlencoded + +datatopost=ohthatsfunyesyes + + +