From b2331f3eeabf6d3d48476040ad8a058c282ddba0 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Tue, 10 Sep 2024 14:08:17 +0200 Subject: [PATCH] request: on shutdown send, proceed normally on timeout When ending an FTP upload, we shut down the connection gracefully, since the server should be notified we had send all bytes. Mostly, this is a NOP without TLS involved. With TLS, close-notify messages should be exchanged. As reported in #14843, not all servers seem to do that. Since it is the server's responsiblity to check it has received everything, we just log the timeout and proceed as if everything is fine. In the receive direction, we still fail the transfer if the server does not shut down its direction properly. Fixes #14843 Reported-by: Rasmus Melchior Jacobsen Closes #14848 --- lib/cfilters.c | 3 ++- lib/ftp.c | 8 +++++--- lib/request.c | 7 +++++++ lib/request.h | 1 + lib/transfer.c | 15 ++++++++++----- lib/transfer.h | 5 +++-- 6 files changed, 28 insertions(+), 11 deletions(-) diff --git a/lib/cfilters.c b/lib/cfilters.c index 59683084d5..daa1877ab2 100644 --- a/lib/cfilters.c +++ b/lib/cfilters.c @@ -210,7 +210,8 @@ CURLcode Curl_conn_shutdown(struct Curl_easy *data, int sockindex, bool *done) else { timeout_ms = Curl_shutdown_timeleft(data->conn, sockindex, &now); if(timeout_ms < 0) { - failf(data, "SSL shutdown timeout"); + /* info message, since this might be regarded as acceptable */ + infof(data, "shutdown timeout"); return CURLE_OPERATION_TIMEDOUT; } } diff --git a/lib/ftp.c b/lib/ftp.c index 8a6a00b677..07ba3bddd8 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -653,12 +653,14 @@ static CURLcode InitiateTransfer(struct Curl_easy *data) /* set the SO_SNDBUF for the secondary socket for those who need it */ Curl_sndbuf_init(conn->sock[SECONDARYSOCKET]); - Curl_xfer_setup2(data, CURL_XFER_SEND, -1, TRUE); + /* FTP upload, shutdown DATA, ignore shutdown errors, as we rely + * on the server response on the CONTROL connection. */ + Curl_xfer_setup2(data, CURL_XFER_SEND, -1, TRUE, TRUE); } else { - /* FTP download: */ + /* FTP download, shutdown, do not ignore errors */ Curl_xfer_setup2(data, CURL_XFER_RECV, - conn->proto.ftpc.retr_size_saved, TRUE); + conn->proto.ftpc.retr_size_saved, TRUE, FALSE); } conn->proto.ftpc.pp.pending_resp = TRUE; /* expect server response */ diff --git a/lib/request.c b/lib/request.c index 0fe4c80117..310e4eac00 100644 --- a/lib/request.c +++ b/lib/request.c @@ -327,6 +327,13 @@ static CURLcode req_flush(struct Curl_easy *data) if(data->req.shutdown) { bool done; result = Curl_xfer_send_shutdown(data, &done); + if(result && data->req.shutdown_err_ignore) { + infof(data, "Shutdown send direction error: %d. Broken server? " + "Proceeding as if everything is ok.", result); + result = CURLE_OK; + done = TRUE; + } + if(result) return result; if(!done) diff --git a/lib/request.h b/lib/request.h index c53c3eb5ae..bb72247788 100644 --- a/lib/request.h +++ b/lib/request.h @@ -151,6 +151,7 @@ struct SingleRequest { negotiation. */ BIT(sendbuf_init); /* sendbuf is initialized */ BIT(shutdown); /* request end will shutdown connection */ + BIT(shutdown_err_ignore); /* errors in shutdown will not fail request */ #ifdef USE_HYPER BIT(bodywritten); #endif diff --git a/lib/transfer.c b/lib/transfer.c index 3ca67d88cc..4c6538e9a5 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -1069,8 +1069,10 @@ static void xfer_setup( bool getheader, /* TRUE if header parsing is wanted */ int writesockindex, /* socket index to write to, it may very well be the same we read from. -1 disables */ - bool shutdown /* shutdown connection at transfer end. Only + bool shutdown, /* shutdown connection at transfer end. Only * supported when sending OR receiving. */ + bool shutdown_err_ignore /* errors during shutdown do not fail the + * transfer */ ) { struct SingleRequest *k = &data->req; @@ -1102,6 +1104,7 @@ static void xfer_setup( k->getheader = getheader; k->size = size; k->shutdown = shutdown; + k->shutdown_err_ignore = shutdown_err_ignore; /* The code sequence below is placed in this function just because all necessary input is not always known in do_complete() as this function may @@ -1126,7 +1129,7 @@ static void xfer_setup( void Curl_xfer_setup_nop(struct Curl_easy *data) { - xfer_setup(data, -1, -1, FALSE, -1, FALSE); + xfer_setup(data, -1, -1, FALSE, -1, FALSE, FALSE); } void Curl_xfer_setup1(struct Curl_easy *data, @@ -1137,18 +1140,20 @@ void Curl_xfer_setup1(struct Curl_easy *data, int recv_index = (send_recv & CURL_XFER_RECV) ? FIRSTSOCKET : -1; int send_index = (send_recv & CURL_XFER_SEND) ? FIRSTSOCKET : -1; DEBUGASSERT((recv_index >= 0) || (recv_size == -1)); - xfer_setup(data, recv_index, recv_size, getheader, send_index, FALSE); + xfer_setup(data, recv_index, recv_size, getheader, send_index, FALSE, FALSE); } void Curl_xfer_setup2(struct Curl_easy *data, int send_recv, curl_off_t recv_size, - bool shutdown) + bool shutdown, + bool shutdown_err_ignore) { int recv_index = (send_recv & CURL_XFER_RECV) ? SECONDARYSOCKET : -1; int send_index = (send_recv & CURL_XFER_SEND) ? SECONDARYSOCKET : -1; DEBUGASSERT((recv_index >= 0) || (recv_size == -1)); - xfer_setup(data, recv_index, recv_size, FALSE, send_index, shutdown); + xfer_setup(data, recv_index, recv_size, FALSE, send_index, + shutdown, shutdown_err_ignore); } CURLcode Curl_xfer_write_resp(struct Curl_easy *data, diff --git a/lib/transfer.h b/lib/transfer.h index 8d6f98d750..87a5a389ff 100644 --- a/lib/transfer.h +++ b/lib/transfer.h @@ -100,12 +100,13 @@ void Curl_xfer_setup1(struct Curl_easy *data, * the amount to receive or -1 if unknown. With `shutdown` being * set, the transfer is only allowed to either send OR receive * and the socket 2 connection will be shutdown at the end of - * the transfer. An unclean shutdown will fail the transfer. + * the transfer. An unclean shutdown will fail the transfer + * unless `shutdown_err_ignore` is TRUE. */ void Curl_xfer_setup2(struct Curl_easy *data, int send_recv, curl_off_t recv_size, - bool shutdown); + bool shutdown, bool shutdown_err_ignore); /** * Multi has set transfer to DONE. Last chance to trigger