From 513c5b1a15a7d8cc7df05fbfddb6e02e52e75ec8 Mon Sep 17 00:00:00 2001
From: David Anderson <davea@ssl.berkeley.edu>
Date: Tue, 6 Sep 2022 12:49:19 -0700
Subject: [PATCH] client: close file descriptors if Curl initialization fails. 
    HTTP_OP::libcurl_exec() opens input and/or output files.     In the normal
 case these eventually are closed in handle_messages()     when the HTTP op
 finishes.     But if the call to curl_multi_add_handle() returns an error    
 (which I believe happens on a connect failure, e.g. if the project is down)  
   then we need to close the files.     We weren't doing this, possibly
 leading to running out of file descriptors.

---
 client/http_curl.cpp | 40 ++++++++++++++++------------------------
 client/http_curl.h   |  4 +---
 2 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/client/http_curl.cpp b/client/http_curl.cpp
index d3abd3769f..4d754f3b5b 100644
--- a/client/http_curl.cpp
+++ b/client/http_curl.cpp
@@ -95,18 +95,8 @@ size_t libcurl_write(void *ptr, size_t size, size_t nmemb, HTTP_OP* phop) {
 }
 
 size_t libcurl_read(void *ptr, size_t size, size_t nmemb, HTTP_OP* phop) {
-    // OK here's the deal -- phop points to the calling object,
-    // which has already pre-opened the file.  we'll want to
-    // use pByte as a pointer for fseek calls into the file, and
-    // write out size*nmemb # of bytes to ptr
-
-    // take the stream param as a FILE* and write to disk
-    // if (pByte) delete [] pByte;
-    // pByte = new unsigned char[content_length];
-    // memset(pByte, 0x00, content_length); // may as will initialize it!
-
-    // note that fileIn was opened earlier,
-    // go to lSeek from the top and read from there
+    // read data from inFile (or from header if present)
+    // and move to buffer for Curl to send it.
     //
     size_t stSend = size * nmemb;
     int stRead = 0;
@@ -264,7 +254,6 @@ HTTP_OP::HTTP_OP() {
     curlEasy = NULL;
     pcurlFormStart = NULL;
     pcurlFormEnd = NULL;
-    pByte = NULL;
     lSeek = 0;
     xfer_speed = 0;
     is_background = false;
@@ -273,7 +262,7 @@ HTTP_OP::HTTP_OP() {
 
 HTTP_OP::~HTTP_OP() {
     close_socket();
-    close_file();
+    close_files();
 }
 
 // Initialize HTTP GET operation;
@@ -406,7 +395,11 @@ static int set_cloexec(void*, curl_socket_t fd, curlsocktype purpose) {
 }
 #endif
 
-// the following will do an HTTP GET or POST using libcurl
+// Initiate an HTTP GET or POST using libcurl.
+// Open input/output files as needed.
+// If error, close these before returning.
+// On success, we'll call handle_messages() in response
+// to select() on the socket, and eventually close the files there.
 //
 int HTTP_OP::libcurl_exec(
     const char* url, const char* in, const char* out, double offset,
@@ -608,6 +601,7 @@ int HTTP_OP::libcurl_exec(
                 msg_printf(NULL, MSG_INTERNAL_ERROR, "No HTTP input file %s", infile);
                 http_op_retval = ERR_FOPEN;
                 http_op_state = HTTP_STATE_DONE;
+                close_files();
                 return ERR_FOPEN;
             }
         }
@@ -652,7 +646,6 @@ int HTTP_OP::libcurl_exec(
 
         curl_off_t fs = (curl_off_t) content_length;
 
-        pByte = NULL;
         lSeek = 0;    // initialize the vars we're going to use for byte transfers
 
         // we can make the libcurl_read "fancier" in the future,
@@ -710,6 +703,7 @@ int HTTP_OP::libcurl_exec(
         msg_printf(0, MSG_INTERNAL_ERROR,
             "Couldn't add curlEasy handle to curlMulti"
         );
+        close_files();
         return ERR_HTTP_TRANSIENT;
         // returns 0 (CURLM_OK) on successful handle creation
     }
@@ -893,7 +887,9 @@ void HTTP_OP::close_socket() {
     }
 }
 
-void HTTP_OP::close_file() {
+// close input and output files
+//
+void HTTP_OP::close_files() {
     if (fileIn) {
         fclose(fileIn);
         fileIn = NULL;
@@ -902,10 +898,6 @@ void HTTP_OP::close_file() {
         fclose(fileOut);
         fileOut = NULL;
     }
-    if (pByte) { //free any read memory used
-        delete [] pByte;
-        pByte = NULL;
-    }
 }
 
 void HTTP_OP_SET::get_fdset(FDSET_GROUP& fg) {
@@ -1046,12 +1038,12 @@ void HTTP_OP::handle_messages(CURLMsg *pcurlMsg) {
         }
     }
 
-    // close files and "sockets" (i.e. libcurl handles)
+    // close in/out files and "sockets" (i.e. libcurl handles)
     //
-    close_file();
+    close_files();
     close_socket();
 
-    // finally remove the tmpfile if not explicitly set
+    // remove the output file if it's a temp
     //
     if (bTempOutfile) {
         boinc_delete_file(outfile);
diff --git a/client/http_curl.h b/client/http_curl.h
index 551e18a993..8c47cba315 100644
--- a/client/http_curl.h
+++ b/client/http_curl.h
@@ -76,8 +76,6 @@ public:
         // a pointer to a form item for POST
     struct curl_httppost *pcurlFormEnd;
         // a pointer to a form item for POST
-    unsigned char* pByte;
-        // pointer to bytes for reading via libcurl_read function
 
     // request message stuff
     //
@@ -148,7 +146,7 @@ public:
     void init(PROJECT*);
     int get_ip_addr(int &ip_addr);
     void close_socket();
-    void close_file();
+    void close_files();
     void update_speed();
     void set_speed_limit(bool is_upload, double bytes_sec);
     void handle_messages(CURLMsg*);