From 77b0571cdc941a974c50dc7ee334af0b41288cd8 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 13 Mar 2024 11:42:17 +0100 Subject: [PATCH] http: revisit http_perhapsrewind() - use facilities provided by client readers better - work also for non-uploading requests like GET/HEAD - update documentation Closes #13117 --- lib/cf-h1-proxy.c | 6 ++ lib/cf-h2-proxy.c | 11 +++- lib/http.c | 152 +++++++++++++++++----------------------------- 3 files changed, 73 insertions(+), 96 deletions(-) diff --git a/lib/cf-h1-proxy.c b/lib/cf-h1-proxy.c index 57cf9a1ae1..ed6322c10d 100644 --- a/lib/cf-h1-proxy.c +++ b/lib/cf-h1-proxy.c @@ -237,6 +237,8 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf, http_minor = (cf->conn->http_proxy.proxytype == CURLPROXY_HTTP_1_0) ? 0 : 1; result = Curl_h1_req_write_head(req, http_minor, &ts->request_data); + if(!result) + result = Curl_creader_set_null(data); out: if(result) @@ -749,6 +751,10 @@ static CURLcode start_CONNECT(struct Curl_cfilter *cf, if(result) goto error; + result = Curl_creader_set_null(data); + if(result) + goto error; + sendtask = hyper_clientconn_send(client, req); if(!sendtask) { failf(data, "hyper_clientconn_send"); diff --git a/lib/cf-h2-proxy.c b/lib/cf-h2-proxy.c index f8f2f3c417..78dc222fa8 100644 --- a/lib/cf-h2-proxy.c +++ b/lib/cf-h2-proxy.c @@ -38,6 +38,7 @@ #include "http2.h" #include "http_proxy.h" #include "multiif.h" +#include "sendf.h" #include "cf-h2-proxy.h" /* The last 3 #include files should be in this order */ @@ -954,6 +955,9 @@ static CURLcode submit_CONNECT(struct Curl_cfilter *cf, struct httpreq *req = NULL; result = Curl_http_proxy_create_CONNECT(&req, cf, data, 2); + if(result) + goto out; + result = Curl_creader_set_null(data); if(result) goto out; @@ -1125,7 +1129,12 @@ static CURLcode cf_h2_proxy_connect(struct Curl_cfilter *cf, out: *done = (result == CURLE_OK) && (ts->state == H2_TUNNEL_ESTABLISHED); - cf->connected = *done; + if(*done) { + cf->connected = TRUE; + /* The real request will follow the CONNECT, reset request partially */ + Curl_req_soft_reset(&data->req, data); + Curl_client_reset(data); + } CF_DATA_RESTORE(cf, save); return result; } diff --git a/lib/http.c b/lib/http.c index 52ae1ae69d..1d741fbd8d 100644 --- a/lib/http.c +++ b/lib/http.c @@ -405,123 +405,88 @@ static bool pickoneauth(struct auth *pick, unsigned long mask) /* * http_perhapsrewind() * - * If we are doing POST or PUT { - * If we have more data to send { - * If we are doing NTLM { - * Keep sending since we must not disconnect - * } - * else { - * If there is more than just a little data left to send, close - * the current connection by force. - * } - * } - * If we have sent any data { - * If we don't have track of all the data { - * call app to tell it to rewind - * } - * else { - * rewind internally so that the operation can restart fine - * } - * } - * } + * The current request needs to be done again - maybe due to a follow + * or authentication negotiation. Check if: + * 1) a rewind of the data sent to the server is necessary + * 2) the current transfer should continue or be stopped early */ static CURLcode http_perhapsrewind(struct Curl_easy *data, struct connectdata *conn) { - struct HTTP *http = data->req.p.http; - curl_off_t bytessent; + curl_off_t bytessent = data->req.writebytecount; curl_off_t expectsend = Curl_creader_total_length(data); + curl_off_t upload_remain = (expectsend >= 0)? (expectsend - bytessent) : -1; + bool little_upload_remains = (upload_remain >= 0 && upload_remain < 2000); + bool needs_rewind = Curl_creader_needs_rewind(data); + /* By default, we'd like to abort the transfer when little or + * unknown amount remains. But this may be overridden by authentications + * further below! */ + bool abort_upload = (!data->req.upload_done && !little_upload_remains); + const char *ongoing_auth = NULL; + + /* We need a rewind before uploading client read data again. The + * checks below just influence of the upload is to be continued + * or aborted early. + * This depends on how much remains to be sent and in what state + * the authentication is. Some auth schemes such as NTLM do not work + * for a new connection. */ + if(needs_rewind) { + infof(data, "Need to rewind upload for next request"); + Curl_creader_set_rewind(data, TRUE); + } - if(!http) - /* If this is still NULL, we have not reach very far and we can safely - skip this rewinding stuff */ - return CURLE_OK; - - if(!expectsend) - /* not sending any body */ + if(conn->bits.close) + /* If we already decided to close this connection, we cannot veto. */ return CURLE_OK; - if(!conn->bits.protoconnstart) - /* HTTP CONNECT in progress: there is no body */ - expectsend = 0; - - bytessent = data->req.writebytecount; - Curl_creader_set_rewind(data, FALSE); - - if((expectsend == -1) || (expectsend > bytessent)) { + if(abort_upload) { + /* We'd like to abort the upload - but should we? */ #if defined(USE_NTLM) - /* There is still data left to send */ if((data->state.authproxy.picked == CURLAUTH_NTLM) || (data->state.authhost.picked == CURLAUTH_NTLM) || (data->state.authproxy.picked == CURLAUTH_NTLM_WB) || (data->state.authhost.picked == CURLAUTH_NTLM_WB)) { - if(((expectsend - bytessent) < 2000) || - (conn->http_ntlm_state != NTLMSTATE_NONE) || + ongoing_auth = "NTML"; + if((conn->http_ntlm_state != NTLMSTATE_NONE) || (conn->proxy_ntlm_state != NTLMSTATE_NONE)) { - /* The NTLM-negotiation has started *OR* there is just a little (<2K) - data left to send, keep on sending. */ - - /* rewind data when completely done sending! */ - if(!data->req.authneg && (conn->writesockfd != CURL_SOCKET_BAD)) { - Curl_creader_set_rewind(data, TRUE); - infof(data, "Rewind stream before next send"); - } - - return CURLE_OK; + /* The NTLM-negotiation has started, keep on sending. + * Need to do further work on same connection */ + abort_upload = FALSE; } - - if(conn->bits.close) - /* this is already marked to get closed */ - return CURLE_OK; - - infof(data, "NTLM send, close instead of sending %" - CURL_FORMAT_CURL_OFF_T " bytes", - (curl_off_t)(expectsend - bytessent)); } #endif #if defined(USE_SPNEGO) /* There is still data left to send */ if((data->state.authproxy.picked == CURLAUTH_NEGOTIATE) || (data->state.authhost.picked == CURLAUTH_NEGOTIATE)) { - if(((expectsend - bytessent) < 2000) || - (conn->http_negotiate_state != GSS_AUTHNONE) || + ongoing_auth = "NEGOTIATE"; + if((conn->http_negotiate_state != GSS_AUTHNONE) || (conn->proxy_negotiate_state != GSS_AUTHNONE)) { - /* The NEGOTIATE-negotiation has started *OR* - there is just a little (<2K) data left to send, keep on sending. */ - - /* rewind data when completely done sending! */ - if(!data->req.authneg && (conn->writesockfd != CURL_SOCKET_BAD)) { - Curl_creader_set_rewind(data, TRUE); - infof(data, "Rewind stream before next send"); - } - - return CURLE_OK; + /* The NEGOTIATE-negotiation has started, keep on sending. + * Need to do further work on same connection */ + abort_upload = FALSE; } - - if(conn->bits.close) - /* this is already marked to get closed */ - return CURLE_OK; - - infof(data, "NEGOTIATE send, close instead of sending %" - CURL_FORMAT_CURL_OFF_T " bytes", - (curl_off_t)(expectsend - bytessent)); } #endif + } - /* This is not NEGOTIATE/NTLM or many bytes left to send: close */ + if(abort_upload) { + if(upload_remain >= 0) + infof(data, "%s%sclose instead of sending %" + CURL_FORMAT_CURL_OFF_T " more bytes", + ongoing_auth? ongoing_auth : "", + ongoing_auth? " send, " : "", + upload_remain); + else + infof(data, "%s%sclose instead of sending unknown amount " + "of more bytes", + ongoing_auth? ongoing_auth : "", + ongoing_auth? " send, " : ""); + /* We decided to abort the ongoing transfer */ streamclose(conn, "Mid-auth HTTP and much data left to send"); + /* FIXME: questionable manipulation here, can we do this differently? */ data->req.size = 0; /* don't download any more than 0 bytes */ - - /* There still is data left to send, but this connection is marked for - closure so we can safely do the rewind right now */ - } - - if(Curl_creader_needs_rewind(data)) { - /* mark for rewind since if we already sent something */ - Curl_creader_set_rewind(data, TRUE); - infof(data, "Please rewind output before next send"); } - return CURLE_OK; } @@ -575,13 +540,10 @@ CURLcode Curl_http_auth_act(struct Curl_easy *data) #endif if(pickhost || pickproxy) { - if((data->state.httpreq != HTTPREQ_GET) && - (data->state.httpreq != HTTPREQ_HEAD) && - !Curl_creader_will_rewind(data)) { - result = http_perhapsrewind(data, conn); - if(result) - return result; - } + result = http_perhapsrewind(data, conn); + if(result) + return result; + /* In case this is GSS auth, the newurl field is already allocated so we must make sure to free it before allocating a new one. As figured out in bug #2284386 */ -- 2.47.3