From 1b3973180b6b7f1023240a67d3acdb11b534bf03 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Tue, 22 Nov 2022 08:25:50 +0100 Subject: [PATCH] lib: rewind BEFORE request instead of AFTER previous This makes a big difference for cases when the rewind is not actually necessary to perofm (for example HTTP response code 301 converts to GET) and therefore the rewind can be avoided. In particular for situations when that rewind fails, for example when reading from a pipe or similar. Reported-by: Ali Utku Selen Fixes #9735 Closes #9958 --- lib/http.c | 30 +++++++------- lib/http2.h | 2 +- lib/http_proxy.c | 3 -- lib/multi.c | 93 ++++++++++++++++++++++++++++++++++++++++++- lib/transfer.c | 100 ++--------------------------------------------- lib/transfer.h | 1 - lib/urldata.h | 7 ++-- 7 files changed, 115 insertions(+), 121 deletions(-) diff --git a/lib/http.c b/lib/http.c index 1e23f901bf..105e8cf8cf 100644 --- a/lib/http.c +++ b/lib/http.c @@ -576,7 +576,7 @@ static CURLcode http_perhapsrewind(struct Curl_easy *data, } } - conn->bits.rewindaftersend = FALSE; /* default */ + data->state.rewindbeforesend = FALSE; /* default */ if((expectsend == -1) || (expectsend > bytessent)) { #if defined(USE_NTLM) @@ -593,8 +593,8 @@ static CURLcode http_perhapsrewind(struct Curl_easy *data, /* rewind data when completely done sending! */ if(!conn->bits.authneg && (conn->writesockfd != CURL_SOCKET_BAD)) { - conn->bits.rewindaftersend = TRUE; - infof(data, "Rewind stream after send"); + data->state.rewindbeforesend = TRUE; + infof(data, "Rewind stream before next send"); } return CURLE_OK; @@ -621,8 +621,8 @@ static CURLcode http_perhapsrewind(struct Curl_easy *data, /* rewind data when completely done sending! */ if(!conn->bits.authneg && (conn->writesockfd != CURL_SOCKET_BAD)) { - conn->bits.rewindaftersend = TRUE; - infof(data, "Rewind stream after send"); + data->state.rewindbeforesend = TRUE; + infof(data, "Rewind stream before next send"); } return CURLE_OK; @@ -646,9 +646,11 @@ static CURLcode http_perhapsrewind(struct Curl_easy *data, closure so we can safely do the rewind right now */ } - if(bytessent) - /* we rewind now at once since if we already sent something */ - return Curl_readrewind(data); + if(bytessent) { + /* mark for rewind since if we already sent something */ + data->state.rewindbeforesend = TRUE; + infof(data, "Please rewind output before next send"); + } return CURLE_OK; } @@ -705,7 +707,7 @@ CURLcode Curl_http_auth_act(struct Curl_easy *data) if(pickhost || pickproxy) { if((data->state.httpreq != HTTPREQ_GET) && (data->state.httpreq != HTTPREQ_HEAD) && - !conn->bits.rewindaftersend) { + !data->state.rewindbeforesend) { result = http_perhapsrewind(data, conn); if(result) return result; @@ -4096,7 +4098,7 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, if(k->httpcode >= 300) { if((!conn->bits.authneg) && !conn->bits.close && - !conn->bits.rewindaftersend) { + !data->state.rewindbeforesend) { /* * General treatment of errors when about to send data. Including : * "417 Expectation Failed", while waiting for 100-continue. @@ -4106,7 +4108,7 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, * something else should've considered the big picture and we * avoid this check. * - * rewindaftersend indicates that something has told libcurl to + * rewindbeforesend indicates that something has told libcurl to * continue sending even if it gets discarded */ @@ -4155,9 +4157,9 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, } } - if(conn->bits.rewindaftersend) { - /* We rewind after a complete send, so thus we continue - sending now */ + if(data->state.rewindbeforesend && + (conn->writesockfd != CURL_SOCKET_BAD)) { + /* We rewind before next send, continue sending now */ infof(data, "Keep sending data to get tossed away"); k->keepon |= KEEP_SEND; } diff --git a/lib/http2.h b/lib/http2.h index f0390596ce..966bf75da0 100644 --- a/lib/http2.h +++ b/lib/http2.h @@ -73,7 +73,7 @@ bool Curl_h2_http_1_1_error(struct Curl_easy *data); #define Curl_http2_init_state(x) #define Curl_http2_init_userset(x) #define Curl_http2_done(x,y) -#define Curl_http2_done_sending(x,y) +#define Curl_http2_done_sending(x,y) (void)y #define Curl_http2_add_child(x, y, z) #define Curl_http2_remove_child(x, y) #define Curl_http2_cleanup_dependencies(x) diff --git a/lib/http_proxy.c b/lib/http_proxy.c index db851e0c80..53810b2838 100644 --- a/lib/http_proxy.c +++ b/lib/http_proxy.c @@ -204,9 +204,6 @@ static void tunnel_go_state(struct Curl_cfilter *cf, case TUNNEL_ESTABLISHED: infof(data, "CONNECT phase completed"); - if(cf->conn) - /* make sure this isn't set for the document request */ - cf->conn->bits.rewindaftersend = FALSE; data->state.authproxy.done = TRUE; data->state.authproxy.multipass = FALSE; /* FALLTHROUGH */ diff --git a/lib/multi.c b/lib/multi.c index 0e60cf3a57..b96ee7c7ea 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1729,6 +1729,90 @@ static CURLcode protocol_connect(struct Curl_easy *data, return result; /* pass back status */ } +/* + * readrewind() rewinds the read stream. This is typically used for HTTP + * POST/PUT with multi-pass authentication when a sending was denied and a + * resend is necessary. + */ +static CURLcode readrewind(struct Curl_easy *data) +{ + struct connectdata *conn = data->conn; + curl_mimepart *mimepart = &data->set.mimepost; + DEBUGASSERT(conn); + + data->state.rewindbeforesend = FALSE; /* we rewind now */ + + /* explicitly switch off sending data on this connection now since we are + about to restart a new transfer and thus we want to avoid inadvertently + sending more data on the existing connection until the next transfer + starts */ + data->req.keepon &= ~KEEP_SEND; + + /* We have sent away data. If not using CURLOPT_POSTFIELDS or + CURLOPT_HTTPPOST, call app to rewind + */ + if(conn->handler->protocol & PROTO_FAMILY_HTTP) { + struct HTTP *http = data->req.p.http; + + if(http->sendit) + mimepart = http->sendit; + } + if(data->set.postfields || + (data->state.httpreq == HTTPREQ_GET) || + (data->state.httpreq == HTTPREQ_HEAD)) + ; /* no need to rewind */ + else if(data->state.httpreq == HTTPREQ_POST_MIME || + data->state.httpreq == HTTPREQ_POST_FORM) { + CURLcode result = Curl_mime_rewind(mimepart); + if(result) { + failf(data, "Cannot rewind mime/post data"); + return result; + } + } + else { + if(data->set.seek_func) { + int err; + + Curl_set_in_callback(data, true); + err = (data->set.seek_func)(data->set.seek_client, 0, SEEK_SET); + Curl_set_in_callback(data, false); + if(err) { + failf(data, "seek callback returned error %d", (int)err); + return CURLE_SEND_FAIL_REWIND; + } + } + else if(data->set.ioctl_func) { + curlioerr err; + + Curl_set_in_callback(data, true); + err = (data->set.ioctl_func)(data, CURLIOCMD_RESTARTREAD, + data->set.ioctl_client); + Curl_set_in_callback(data, false); + infof(data, "the ioctl callback returned %d", (int)err); + + if(err) { + failf(data, "ioctl callback returned error %d", (int)err); + return CURLE_SEND_FAIL_REWIND; + } + } + else { + /* If no CURLOPT_READFUNCTION is used, we know that we operate on a + given FILE * stream and we can actually attempt to rewind that + ourselves with fseek() */ + if(data->state.fread_func == (curl_read_callback)fread) { + if(-1 != fseek(data->state.in, 0, SEEK_SET)) + /* successful rewind */ + return CURLE_OK; + } + + /* no callback set or failure above, makes us fail at once */ + failf(data, "necessary data rewind wasn't possible"); + return CURLE_SEND_FAIL_REWIND; + } + } + return CURLE_OK; +} + /* * Curl_preconnect() is called immediately before a connect starts. When a * redirect is followed, this is then called multiple times during a single @@ -1741,6 +1825,7 @@ CURLcode Curl_preconnect(struct Curl_easy *data) if(!data->state.buffer) return CURLE_OUT_OF_MEMORY; } + return CURLE_OK; } @@ -1997,7 +2082,10 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, break; case MSTATE_PROTOCONNECT: - if(data->conn->bits.reuse) { + if(data->state.rewindbeforesend) + result = readrewind(data); + + if(!result && data->conn->bits.reuse) { /* ftp seems to hang when protoconnect on reused connection * since we handle PROTOCONNECT in general inside the filers, it * seems wrong to restart this on a reused connection. */ @@ -2005,7 +2093,8 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi, rc = CURLM_CALL_MULTI_PERFORM; break; } - result = protocol_connect(data, &protocol_connected); + if(!result) + result = protocol_connect(data, &protocol_connected); if(!result && !protocol_connected) /* switch to waiting state */ multistate(data, MSTATE_PROTOCONNECTING); diff --git a/lib/transfer.c b/lib/transfer.c index c10f7e38d3..ba0410fc51 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -363,88 +363,6 @@ CURLcode Curl_fillreadbuffer(struct Curl_easy *data, size_t bytes, return CURLE_OK; } - -/* - * Curl_readrewind() rewinds the read stream. This is typically used for HTTP - * POST/PUT with multi-pass authentication when a sending was denied and a - * resend is necessary. - */ -CURLcode Curl_readrewind(struct Curl_easy *data) -{ - struct connectdata *conn = data->conn; - curl_mimepart *mimepart = &data->set.mimepost; - - conn->bits.rewindaftersend = FALSE; /* we rewind now */ - - /* explicitly switch off sending data on this connection now since we are - about to restart a new transfer and thus we want to avoid inadvertently - sending more data on the existing connection until the next transfer - starts */ - data->req.keepon &= ~KEEP_SEND; - - /* We have sent away data. If not using CURLOPT_POSTFIELDS or - CURLOPT_HTTPPOST, call app to rewind - */ - if(conn->handler->protocol & PROTO_FAMILY_HTTP) { - struct HTTP *http = data->req.p.http; - - if(http->sendit) - mimepart = http->sendit; - } - if(data->set.postfields) - ; /* do nothing */ - else if(data->state.httpreq == HTTPREQ_POST_MIME || - data->state.httpreq == HTTPREQ_POST_FORM) { - CURLcode result = Curl_mime_rewind(mimepart); - if(result) { - failf(data, "Cannot rewind mime/post data"); - return result; - } - } - else { - if(data->set.seek_func) { - int err; - - Curl_set_in_callback(data, true); - err = (data->set.seek_func)(data->set.seek_client, 0, SEEK_SET); - Curl_set_in_callback(data, false); - if(err) { - failf(data, "seek callback returned error %d", (int)err); - return CURLE_SEND_FAIL_REWIND; - } - } - else if(data->set.ioctl_func) { - curlioerr err; - - Curl_set_in_callback(data, true); - err = (data->set.ioctl_func)(data, CURLIOCMD_RESTARTREAD, - data->set.ioctl_client); - Curl_set_in_callback(data, false); - infof(data, "the ioctl callback returned %d", (int)err); - - if(err) { - failf(data, "ioctl callback returned error %d", (int)err); - return CURLE_SEND_FAIL_REWIND; - } - } - else { - /* If no CURLOPT_READFUNCTION is used, we know that we operate on a - given FILE * stream and we can actually attempt to rewind that - ourselves with fseek() */ - if(data->state.fread_func == (curl_read_callback)fread) { - if(-1 != fseek(data->state.in, 0, SEEK_SET)) - /* successful rewind */ - return CURLE_OK; - } - - /* no callback set or failure above, makes us fail at once */ - failf(data, "necessary data rewind wasn't possible"); - return CURLE_SEND_FAIL_REWIND; - } - } - return CURLE_OK; -} - static int data_pending(struct Curl_easy *data) { struct connectdata *conn = data->conn; @@ -895,11 +813,6 @@ CURLcode Curl_done_sending(struct Curl_easy *data, Curl_http2_done_sending(data, conn); Curl_quic_done_sending(data); - if(conn->bits.rewindaftersend) { - CURLcode result = Curl_readrewind(data); - if(result) - return result; - } return CURLE_OK; } @@ -1383,7 +1296,6 @@ int Curl_single_getsock(struct Curl_easy *data, /* don't include HOLD and PAUSE connections */ if((data->req.keepon & KEEP_SENDBITS) == KEEP_SEND) { - if((conn->sockfd != conn->writesockfd) || bitmap == GETSOCK_BLANK) { /* only if they are not the same socket and we have a readable @@ -1925,14 +1837,10 @@ CURLcode Curl_retry_request(struct Curl_easy *data, char **url) transferred! */ - if(conn->handler->protocol&PROTO_FAMILY_HTTP) { - if(data->req.writebytecount) { - CURLcode result = Curl_readrewind(data); - if(result) { - Curl_safefree(*url); - return result; - } - } + if((conn->handler->protocol&PROTO_FAMILY_HTTP) && + data->req.writebytecount) { + data->state.rewindbeforesend = TRUE; + infof(data, "state.rewindbeforesend = TRUE"); } } return CURLE_OK; diff --git a/lib/transfer.h b/lib/transfer.h index 65fe68e812..4092508729 100644 --- a/lib/transfer.h +++ b/lib/transfer.h @@ -50,7 +50,6 @@ CURLcode Curl_readwrite(struct connectdata *conn, bool *comeback); int Curl_single_getsock(struct Curl_easy *data, struct connectdata *conn, curl_socket_t *socks); -CURLcode Curl_readrewind(struct Curl_easy *data); CURLcode Curl_fillreadbuffer(struct Curl_easy *data, size_t bytes, size_t *nreadp); CURLcode Curl_retry_request(struct Curl_easy *data, char **url); diff --git a/lib/urldata.h b/lib/urldata.h index 48b281f3ad..522f534a50 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -516,10 +516,6 @@ struct ConnectBits { that we are creating a request with an auth header, but it is not the final request in the auth negotiation. */ - BIT(rewindaftersend);/* TRUE when the sending couldn't be stopped even - though it will be discarded. When the whole send - operation is done, we must call the data rewind - callback. */ #ifndef CURL_DISABLE_FTP BIT(ftp_use_epsv); /* As set with CURLOPT_FTP_USE_EPSV, but if we find out EPSV doesn't work we disable it for the forthcoming @@ -1483,6 +1479,9 @@ struct UrlState { BIT(url_alloc); /* URL string is malloc()'ed */ BIT(referer_alloc); /* referer string is malloc()ed */ BIT(wildcard_resolve); /* Set to true if any resolve change is a wildcard */ + BIT(rewindbeforesend);/* TRUE when the sending couldn't be stopped even + though it will be discarded. We must call the data + rewind callback before trying to send again. */ }; /* -- 2.47.2