From 88982574462c719d1c648b84f533f7a4dab8c48d Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Tue, 19 Sep 2023 12:31:31 +0200 Subject: [PATCH] lib: disambiguate Curl_client_write flag semantics - use CLIENTWRITE_BODY *only* when data is actually body data - add CLIENTWRITE_INFO for meta data that is *not* a HEADER - debug assertions that BODY/INFO/HEADER is not used mixed - move `data->set.include_header` check into Curl_client_write so protocol handlers no longer have to care - add special in FTP for `data->set.include_header` for historic, backward compatible reasons - move unpausing of client writes from easy.c to sendf.c, so that code is in one place and can forward flags correctly Closes #11885 --- lib/c-hyper.c | 4 --- lib/cf-h1-proxy.c | 1 - lib/easy.c | 31 +++-------------------- lib/ftp.c | 33 ++++++++++++++++++++---- lib/http.c | 3 --- lib/pingpong.c | 2 +- lib/sendf.c | 64 +++++++++++++++++++++++++++++++++++++---------- lib/sendf.h | 31 +++++++++++++++++------ lib/urldata.h | 1 + 9 files changed, 107 insertions(+), 63 deletions(-) diff --git a/lib/c-hyper.c b/lib/c-hyper.c index 61ca29a3ac..47dadfe4ce 100644 --- a/lib/c-hyper.c +++ b/lib/c-hyper.c @@ -174,8 +174,6 @@ static int hyper_each_header(void *userdata, if(!data->state.hconnect || !data->set.suppress_connect_headers) { writetype = CLIENTWRITE_HEADER; - if(data->set.include_header) - writetype |= CLIENTWRITE_BODY; if(data->state.hconnect) writetype |= CLIENTWRITE_CONNECT; if(data->req.httpcode/100 == 1) @@ -314,8 +312,6 @@ static CURLcode status_line(struct Curl_easy *data, if(!data->state.hconnect || !data->set.suppress_connect_headers) { writetype = CLIENTWRITE_HEADER|CLIENTWRITE_STATUS; - if(data->set.include_header) - writetype |= CLIENTWRITE_BODY; result = Curl_client_write(data, writetype, Curl_dyn_ptr(&data->state.headerb), len); if(result) diff --git a/lib/cf-h1-proxy.c b/lib/cf-h1-proxy.c index e9bc13d2a8..23cf85c459 100644 --- a/lib/cf-h1-proxy.c +++ b/lib/cf-h1-proxy.c @@ -579,7 +579,6 @@ static CURLcode recv_CONNECT_resp(struct Curl_cfilter *cf, if(!data->set.suppress_connect_headers) { /* send the header to the callback */ int writetype = CLIENTWRITE_HEADER | CLIENTWRITE_CONNECT | - (data->set.include_header ? CLIENTWRITE_BODY : 0) | (ts->headerlines == 1 ? CLIENTWRITE_STATUS : 0); result = Curl_client_write(data, writetype, linep, perline); diff --git a/lib/easy.c b/lib/easy.c index 03195481f9..3cb2dfb3fe 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -1118,34 +1118,9 @@ CURLcode curl_easy_pause(struct Curl_easy *data, int action) if(!(newstate & KEEP_RECV_PAUSE)) { Curl_conn_ev_data_pause(data, FALSE); - - if(data->state.tempcount) { - /* there are buffers for sending that can be delivered as the receive - pausing is lifted! */ - unsigned int i; - unsigned int count = data->state.tempcount; - struct tempbuf writebuf[3]; /* there can only be three */ - - /* copy the structs to allow for immediate re-pausing */ - for(i = 0; i < data->state.tempcount; i++) { - writebuf[i] = data->state.tempwrite[i]; - Curl_dyn_init(&data->state.tempwrite[i].b, DYN_PAUSE_BUFFER); - } - data->state.tempcount = 0; - - for(i = 0; i < count; i++) { - /* even if one function returns error, this loops through and frees - all buffers */ - if(!result) - result = Curl_client_write(data, writebuf[i].type, - Curl_dyn_ptr(&writebuf[i].b), - Curl_dyn_len(&writebuf[i].b)); - Curl_dyn_free(&writebuf[i].b); - } - - if(result) - return result; - } + result = Curl_client_unpause(data); + if(result) + return result; } #ifdef USE_HYPER diff --git a/lib/ftp.c b/lib/ftp.c index e24ae9c15d..199a85447d 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -2067,6 +2067,31 @@ static bool ftp_213_date(const char *p, int *year, int *month, int *day, return TRUE; } +static CURLcode client_write_header(struct Curl_easy *data, + char *buf, size_t blen) +{ + /* Some replies from an FTP server are written to the client + * as CLIENTWRITE_HEADER, formatted as if they came from a + * HTTP conversation. + * In all protocols, CLIENTWRITE_HEADER data is only passed to + * the body write callback when data->set.include_header is set + * via CURLOPT_HEADER. + * For historic reasons, FTP never played this game and expects + * all its HEADERs to do that always. Set that flag during the + * call to Curl_client_write() so it does the right thing. + * + * Notice that we cannot enable this flag for FTP in general, + * as an FTP transfer might involve a HTTP proxy connection and + * headers from CONNECT should not automatically be part of the + * output. */ + CURLcode result; + int save = data->set.include_header; + data->set.include_header = TRUE; + result = Curl_client_write(data, CLIENTWRITE_HEADER, buf, blen); + data->set.include_header = save? TRUE:FALSE; + return result; +} + static CURLcode ftp_state_mdtm_resp(struct Curl_easy *data, int ftpcode) { @@ -2120,8 +2145,7 @@ static CURLcode ftp_state_mdtm_resp(struct Curl_easy *data, tm->tm_hour, tm->tm_min, tm->tm_sec); - result = Curl_client_write(data, CLIENTWRITE_BOTH, headerbuf, - headerbuflen); + result = client_write_header(data, headerbuf, headerbuflen); if(result) return result; } /* end of a ridiculous amount of conditionals */ @@ -2331,7 +2355,7 @@ static CURLcode ftp_state_size_resp(struct Curl_easy *data, char clbuf[128]; int clbuflen = msnprintf(clbuf, sizeof(clbuf), "Content-Length: %" CURL_FORMAT_CURL_OFF_T "\r\n", filesize); - result = Curl_client_write(data, CLIENTWRITE_BOTH, clbuf, clbuflen); + result = client_write_header(data, clbuf, clbuflen); if(result) return result; } @@ -2365,8 +2389,7 @@ static CURLcode ftp_state_rest_resp(struct Curl_easy *data, #ifdef CURL_FTP_HTTPSTYLE_HEAD if(ftpcode == 350) { char buffer[24]= { "Accept-ranges: bytes\r\n" }; - result = Curl_client_write(data, CLIENTWRITE_BOTH, buffer, - strlen(buffer)); + result = client_write_header(data, buffer, strlen(buffer)); if(result) return result; } diff --git a/lib/http.c b/lib/http.c index c44132f4be..20dd29ca1c 100644 --- a/lib/http.c +++ b/lib/http.c @@ -4238,7 +4238,6 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, /* now, only output this if the header AND body are requested: */ writetype = CLIENTWRITE_HEADER | - (data->set.include_header ? CLIENTWRITE_BODY : 0) | ((k->httpcode/100 == 1) ? CLIENTWRITE_1XX : 0); headerlen = Curl_dyn_len(&data->state.headerb); @@ -4570,8 +4569,6 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, /* * End of header-checks. Write them to the client. */ - if(data->set.include_header) - writetype |= CLIENTWRITE_BODY; if(k->httpcode/100 == 1) writetype |= CLIENTWRITE_1XX; diff --git a/lib/pingpong.c b/lib/pingpong.c index 136985fc92..0081c9ca62 100644 --- a/lib/pingpong.c +++ b/lib/pingpong.c @@ -361,7 +361,7 @@ CURLcode Curl_pp_readresp(struct Curl_easy *data, * for "headers". The response lines can be seen as a kind of * headers. */ - result = Curl_client_write(data, CLIENTWRITE_HEADER, + result = Curl_client_write(data, CLIENTWRITE_INFO, pp->linestart_resp, perline); if(result) return result; diff --git a/lib/sendf.c b/lib/sendf.c index d79ad08fa5..9f30ffc8c8 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -213,6 +213,7 @@ CURLcode Curl_write(struct Curl_easy *data, static CURLcode pausewrite(struct Curl_easy *data, int type, /* what type of data */ + bool paused_body, const char *ptr, size_t len) { @@ -228,7 +229,8 @@ static CURLcode pausewrite(struct Curl_easy *data, if(s->tempcount) { for(i = 0; i< s->tempcount; i++) { - if(s->tempwrite[i].type == type) { + if(s->tempwrite[i].type == type && + !!s->tempwrite[i].paused_body == !!paused_body) { /* data for this type exists */ newtype = FALSE; break; @@ -246,6 +248,7 @@ static CURLcode pausewrite(struct Curl_easy *data, /* store this information in the state struct for later use */ Curl_dyn_init(&s->tempwrite[i].b, DYN_PAUSE_BUFFER); s->tempwrite[i].type = type; + s->tempwrite[i].paused_body = paused_body; s->tempcount++; } @@ -265,6 +268,7 @@ static CURLcode pausewrite(struct Curl_easy *data, */ static CURLcode chop_write(struct Curl_easy *data, int type, + bool skip_body_write, char *optr, size_t olen) { @@ -281,10 +285,12 @@ static CURLcode chop_write(struct Curl_easy *data, /* If reading is paused, append this data to the already held data for this type. */ if(data->req.keepon & KEEP_RECV_PAUSE) - return pausewrite(data, type, ptr, len); + return pausewrite(data, type, !skip_body_write, ptr, len); /* Determine the callback(s) to use. */ - if(type & CLIENTWRITE_BODY) { + if(!skip_body_write && + ((type & CLIENTWRITE_BODY) || + ((type & CLIENTWRITE_HEADER) && data->set.include_header))) { #ifdef USE_WEBSOCKETS if(conn->handler->protocol & (CURLPROTO_WS|CURLPROTO_WSS)) { writebody = Curl_ws_writecb; @@ -294,7 +300,7 @@ static CURLcode chop_write(struct Curl_easy *data, #endif writebody = data->set.fwrite_func; } - if((type & CLIENTWRITE_HEADER) && + if((type & (CLIENTWRITE_HEADER|CLIENTWRITE_INFO)) && (data->set.fwrite_header || data->set.writeheader)) { /* * Write headers to the same callback or to the especially setup @@ -322,7 +328,7 @@ static CURLcode chop_write(struct Curl_easy *data, failf(data, "Write callback asked for PAUSE when not supported"); return CURLE_WRITE_ERROR; } - return pausewrite(data, type, ptr, len); + return pausewrite(data, type, TRUE, ptr, len); } if(wrote != chunklen) { failf(data, "Failure writing output to destination"); @@ -357,13 +363,7 @@ static CURLcode chop_write(struct Curl_easy *data, Curl_set_in_callback(data, false); if(CURL_WRITEFUNC_PAUSE == wrote) - /* here we pass in the HEADER bit only since if this was body as well - then it was passed already and clearly that didn't trigger the - pause, so this is saved for later with the HEADER bit only */ - return pausewrite(data, CLIENTWRITE_HEADER | - (type & (CLIENTWRITE_STATUS|CLIENTWRITE_CONNECT| - CLIENTWRITE_1XX|CLIENTWRITE_TRAILER)), - optr, olen); + return pausewrite(data, type, FALSE, optr, olen); if(wrote != olen) { failf(data, "Failed writing header"); return CURLE_WRITE_ERROR; @@ -397,7 +397,45 @@ CURLcode Curl_client_write(struct Curl_easy *data, len = convert_lineends(data, ptr, len); } #endif - return chop_write(data, type, ptr, len); + /* it is one of those, at least */ + DEBUGASSERT(type & (CLIENTWRITE_BODY|CLIENTWRITE_HEADER|CLIENTWRITE_INFO)); + /* BODY is only BODY */ + DEBUGASSERT(!(type & CLIENTWRITE_BODY) || (type == CLIENTWRITE_BODY)); + /* INFO is only INFO */ + DEBUGASSERT(!(type & CLIENTWRITE_INFO) || (type == CLIENTWRITE_INFO)); + return chop_write(data, type, FALSE, ptr, len); +} + +CURLcode Curl_client_unpause(struct Curl_easy *data) +{ + CURLcode result = CURLE_OK; + + if(data->state.tempcount) { + /* there are buffers for sending that can be delivered as the receive + pausing is lifted! */ + unsigned int i; + unsigned int count = data->state.tempcount; + struct tempbuf writebuf[3]; /* there can only be three */ + + /* copy the structs to allow for immediate re-pausing */ + for(i = 0; i < data->state.tempcount; i++) { + writebuf[i] = data->state.tempwrite[i]; + Curl_dyn_init(&data->state.tempwrite[i].b, DYN_PAUSE_BUFFER); + } + data->state.tempcount = 0; + + for(i = 0; i < count; i++) { + /* even if one function returns error, this loops through and frees + all buffers */ + if(!result) + result = chop_write(data, writebuf[i].type, + !writebuf[i].paused_body, + Curl_dyn_ptr(&writebuf[i].b), + Curl_dyn_len(&writebuf[i].b)); + Curl_dyn_free(&writebuf[i].b); + } + } + return result; } /* diff --git a/lib/sendf.h b/lib/sendf.h index 5ef2b91df7..f55110f963 100644 --- a/lib/sendf.h +++ b/lib/sendf.h @@ -28,18 +28,33 @@ #include "curl_trc.h" - -#define CLIENTWRITE_BODY (1<<0) -#define CLIENTWRITE_HEADER (1<<1) -#define CLIENTWRITE_STATUS (1<<2) /* the first "header" is the status line */ -#define CLIENTWRITE_CONNECT (1<<3) /* a CONNECT response */ -#define CLIENTWRITE_1XX (1<<4) /* a 1xx response */ -#define CLIENTWRITE_TRAILER (1<<5) /* a trailer header */ -#define CLIENTWRITE_BOTH (CLIENTWRITE_BODY|CLIENTWRITE_HEADER) +/** + * Type of data that is being written to the client (application) + * - data written can be eiter BODY or META data + * - META data is either INFO or HEADER + * - INFO is meta information, e.g. not BODY, that cannot be interpreted + * as headers of a response. Example FTP/IMAP pingpong answers. + * - HEADER can have additional bits set (more than one) + * - STATUS special "header", e.g. response status line in HTTP + * - CONNECT header was received during proxying the connection + * - 1XX header is part of an intermediate response, e.g. HTTP 1xx code + * - TRAILER header is trailing response data, e.g. HTTP trailers + * BODY, INFO and HEADER should not be mixed, as this would lead to + * confusion on how to interpret/format/convert the data. + */ +#define CLIENTWRITE_BODY (1<<0) /* non-meta information, BODY */ +#define CLIENTWRITE_INFO (1<<1) /* meta information, not a HEADER */ +#define CLIENTWRITE_HEADER (1<<2) /* meta information, HEADER */ +#define CLIENTWRITE_STATUS (1<<3) /* a special status HEADER */ +#define CLIENTWRITE_CONNECT (1<<4) /* a CONNECT related HEADER */ +#define CLIENTWRITE_1XX (1<<5) /* a 1xx response related HEADER */ +#define CLIENTWRITE_TRAILER (1<<6) /* a trailer HEADER */ CURLcode Curl_client_write(struct Curl_easy *data, int type, char *ptr, size_t len) WARN_UNUSED_RESULT; +CURLcode Curl_client_unpause(struct Curl_easy *data); + /* internal read-function, does plain socket, SSL and krb4 */ CURLcode Curl_read(struct Curl_easy *data, curl_socket_t sockfd, char *buf, size_t buffersize, diff --git a/lib/urldata.h b/lib/urldata.h index c6e69f3db9..82f0f18b1b 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1262,6 +1262,7 @@ struct tempbuf { struct dynbuf b; int type; /* type of the 'tempwrite' buffer as a bitmask that is used with Curl_client_write() */ + BIT(paused_body); /* if PAUSE happend before/during BODY write */ }; /* Timers */