curl tool: fix some more OOM handling

src/main.c

@@ -3110,7 +3110,6 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
   struct ProgressData progressbar;
   struct getout *urlnode;
 
-  struct OutStruct outs;
   struct OutStruct heads;
 
   CURL *curl = NULL;
@@ -3121,8 +3120,10 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
   int i;
 
   errorbuffer[0] = '\0';
-  memset(&outs, 0, sizeof(struct OutStruct));
+  /* default headers output stream is stdout */
   memset(&heads, 0, sizeof(struct OutStruct));
+  heads.stream = stdout;
+  heads.config = config;
 
   memory_tracking_init();
 
@@ -3152,9 +3153,6 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
   }
   config->easy = curl;
 
-  /* Store a pointer to our 'outs' struct used for output writing */
-  config->outs = &outs;
-
   /*
   ** Beyond this point no return'ing from this function allowed.
   ** Jump to label 'quit_curl' in order to abandon this function
@@ -3352,7 +3350,7 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
   /* Single header file for all URLs */
   if(config->headerfile) {
     /* open file for output: */
-    if(strcmp(config->headerfile, "-")) {
+    if(!curlx_strequal(config->headerfile, "-")) {
       FILE *newfile = fopen(config->headerfile, "wb");
       if(!newfile) {
         warnf(config, "Failed to open %s\n", config->headerfile);
@@ -3361,17 +3359,11 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
       }
       else {
         heads.filename = config->headerfile;
-        heads.alloc_filename = FALSE;
+        heads.s_isreg = TRUE;
+        heads.fopened = TRUE;
         heads.stream = newfile;
-        heads.config = config;
       }
     }
-    else {
-      heads.filename = config->headerfile; /* "-" */
-      heads.alloc_filename = FALSE;
-      heads.stream = stdout;
-      heads.config = config;
-    }
   }
 
   /*
@@ -3403,12 +3395,6 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
       continue; /* next URL please */
     }
 
-    /* default output stream is stdout */
-    outs.stream = stdout;
-    outs.config = config;
-    outs.bytes = 0; /* nothing written yet */
-    outs.filename = NULL;
-
     /* save outfile pattern before expansion */
     if(urlnode->outfile) {
       outfiles = strdup(urlnode->outfile);
@@ -3480,6 +3466,7 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
         int infd;
         bool infdopen;
         char *outfile;
+        struct OutStruct outs;
         struct InStruct input;
         struct timeval retrystart;
         curl_off_t uploadfilesize;
@@ -3493,6 +3480,11 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
         infd = STDIN_FILENO;
         uploadfilesize = -1; /* -1 means unknown */
 
+        /* default output stream is stdout */
+        memset(&outs, 0, sizeof(struct OutStruct));
+        outs.stream = stdout;
+        outs.config = config;
+
         if(urls)
           this_url = glob_next_url(urls);
         else if(!i) {
@@ -3563,37 +3555,42 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
             }
           }
 
-          if(config->resume_from_current) {
+          if((urlnode->flags & GETOUT_USEREMOTE)
-            /* We're told to continue from where we are now. Get the
+             && config->content_disposition) {
-               size of the file as it is now and open it for append instead */
+            /* Our header callback sets the filename */
-
+            DEBUGASSERT(!outs.filename);
-            struct_stat fileinfo;
-
-            /* VMS -- Danger, the filesize is only valid for stream files */
-            if(0 == stat(outfile, &fileinfo))
-              /* set offset to current file size: */
-              config->resume_from = fileinfo.st_size;
-            else
-              /* let offset be 0 */
-              config->resume_from = 0;
-          }
-
-          outs.filename = outfile;
-          outs.alloc_filename = FALSE;
-
-          if(config->resume_from) {
-            outs.init = config->resume_from;
-            /* open file for output: */
-            outs.stream = fopen(outfile, config->resume_from?"ab":"wb");
-            if(!outs.stream) {
-              helpf(config->errors, "Can't open '%s'!\n", outfile);
-              res = CURLE_WRITE_ERROR;
-              goto quit_urls;
-            }
           }
           else {
-            outs.stream = NULL; /* open when needed */
+            if(config->resume_from_current) {
-            outs.bytes = 0;     /* reset byte counter */
+              /* We're told to continue from where we are now. Get the size
+                 of the file as it is now and open it for append instead */
+              struct_stat fileinfo;
+              /* VMS -- Danger, the filesize is only valid for stream files */
+              if(0 == stat(outfile, &fileinfo))
+                /* set offset to current file size: */
+                config->resume_from = fileinfo.st_size;
+              else
+                /* let offset be 0 */
+                config->resume_from = 0;
+            }
+
+            if(config->resume_from) {
+              /* open file for output: */
+              FILE *file = fopen(outfile, config->resume_from?"ab":"wb");
+              if(!file) {
+                helpf(config->errors, "Can't open '%s'!\n", outfile);
+                res = CURLE_WRITE_ERROR;
+                goto quit_urls;
+              }
+              outs.fopened = TRUE;
+              outs.stream = file;
+              outs.init = config->resume_from;
+            }
+            else {
+              outs.stream = NULL; /* open when needed */
+            }
+            outs.filename = outfile;
+            outs.s_isreg = TRUE;
           }
         }
 
@@ -4375,8 +4372,7 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
         quit_urls:
 
         /* Set file extended attributes */
-        if(!res && config->xattr &&
+        if(!res && config->xattr && outs.fopened && outs.stream) {
-           outfile && !curlx_strequal(outfile, "-") && outs.stream) {
           int rc = fwrite_xattr(curl, fileno(outs.stream));
           if(rc)
             warnf(config, "Error setting extended attributes: %s\n",
@@ -4384,30 +4380,36 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
         }
 
         /* Close the file */
-        if(outfile && !curlx_strequal(outfile, "-") && outs.stream) {
+        if(outs.fopened && outs.stream) {
           int rc = fclose(outs.stream);
           if(!res && rc) {
             /* something went wrong in the writing process */
             res = CURLE_WRITE_ERROR;
             fprintf(config->errors, "(%d) Failed writing body\n", res);
           }
-          if(outs.alloc_filename)
+        }
-            Curl_safefree(outs.filename);
+        else if(!outs.s_isreg && outs.stream) {
+          /* Dump standard stream buffered data */
+          int rc = fflush(outs.stream);
+          if(!res && rc) {
+            /* something went wrong in the writing process */
+            res = CURLE_WRITE_ERROR;
+            fprintf(config->errors, "(%d) Failed writing body\n", res);
+          }
         }
 
 #ifdef __AMIGA__
-        if(!res) {
+        if(!res && outs.s_isreg && outs.filename) {
           /* Set the url (up to 80 chars) as comment for the file */
           if(strlen(url) > 78)
             url[79] = '\0';
-          SetComment( outs.filename, url);
+          SetComment(outs.filename, url);
         }
 #endif
 
 #ifdef HAVE_UTIME
         /* File time can only be set _after_ the file has been closed */
-        if(!res && config->remote_time &&
+        if(!res && config->remote_time && outs.s_isreg && outs.filename) {
-           outs.filename && !curlx_strequal("-", outs.filename)) {
           /* Ask libcurl if we got a remote file time */
           long filetime = -1;
           curl_easy_getinfo(curl, CURLINFO_FILETIME, &filetime);
@@ -4419,6 +4421,10 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
           }
         }
 #endif
+        /* No more business with this output struct */
+        if(outs.alloc_filename)
+          Curl_safefree(outs.filename);
+        memset(&outs, 0, sizeof(struct OutStruct));
 
         /* Free loop-local allocated memory and close loop-local opened fd */
 
@@ -4437,7 +4443,7 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
 
       } /* loop to the next URL */
 
-      /* Free loop-local allocated memory and close loop-local opened fd */
+      /* Free loop-local allocated memory */
 
       Curl_safefree(uploadfile);
 
@@ -4453,7 +4459,7 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
 
     } /* loop to the next globbed upload file */
 
-    /* Free loop-local allocated memory and close loop-local opened fd */
+    /* Free loop-local allocated memory */
 
     Curl_safefree(outfiles);
 
@@ -4511,12 +4517,10 @@ operate(struct Configurable *config, int argc, argv_item_t argv[])
 
   /* Close function-local opened file descriptors */
 
-  if(config->headerfile) {
+  if(heads.fopened && heads.stream)
-    if(strcmp(heads.filename, "-") && heads.stream)
+    fclose(heads.stream);
-      fclose(heads.stream);
+  if(heads.alloc_filename)
-    if(heads.alloc_filename)
+    Curl_safefree(heads.filename);
-      Curl_safefree(heads.filename);
-  }
 
   if(config->trace_fopened && config->trace_stream)
     fclose(config->trace_stream);

src/tool_cb_hdr.c

@@ -56,6 +56,9 @@ size_t tool_header_cb(void *ptr, size_t size, size_t nmemb, void *userdata)
    */
   size_t failure = (size * nmemb) ? 0 : 1;
 
+  if(!outs->config)
+    return failure;
+
 #ifdef DEBUGBUILD
   if((size * nmemb > (size_t)CURL_MAX_WRITE_SIZE) ||
      (size * nmemb > (size_t)CURL_MAX_HTTP_HEADER)) {
@@ -64,7 +67,8 @@ size_t tool_header_cb(void *ptr, size_t size, size_t nmemb, void *userdata)
   }
 #endif
 
-  if(cb > 20 && checkprefix("Content-disposition:", str)) {
+  if(!outs->filename && (cb > 20) &&
+    checkprefix("Content-disposition:", str)) {
     const char *p = str + 20;
 
     /* look for the 'filename=' parameter
@@ -94,6 +98,9 @@ size_t tool_header_cb(void *ptr, size_t size, size_t nmemb, void *userdata)
       if(filename) {
         outs->filename = filename;
         outs->alloc_filename = TRUE;
+        outs->s_isreg = TRUE;
+        outs->fopened = FALSE;
+        outs->stream = NULL;
         break;
       }
       else

src/tool_cb_wrt.c

@@ -40,8 +40,8 @@
 size_t tool_write_cb(void *buffer, size_t sz, size_t nmemb, void *userdata)
 {
   size_t rc;
-  struct OutStruct *out = userdata;
+  struct OutStruct *outs = userdata;
-  struct Configurable *config = out->config;
+  struct Configurable *config = outs->config;
 
   /*
    * Once that libcurl has called back tool_write_cb() the returned value
@@ -49,47 +49,85 @@ size_t tool_write_cb(void *buffer, size_t sz, size_t nmemb, void *userdata)
    * it does not match then it fails with CURLE_WRITE_ERROR. So at this
    * point returning a value different from sz*nmemb indicates failure.
    */
-  const size_t err_rc = (sz * nmemb) ? 0 : 1;
+  const size_t failure = (sz * nmemb) ? 0 : 1;
+
+  if(!config)
+    return failure;
 
 #ifdef DEBUGBUILD
   if(sz * nmemb > (size_t)CURL_MAX_WRITE_SIZE) {
     warnf(config, "Data size exceeds single call write limit!\n");
-    return err_rc; /* Failure */
+    return failure;
+  }
+
+  {
+    /* Some internal congruency checks on received OutStruct */
+    bool check_fails = FALSE;
+    if(outs->filename) {
+      /* regular file */
+      if(!*outs->filename)
+        check_fails = TRUE;
+      if(!outs->s_isreg)
+        check_fails = TRUE;
+      if(outs->fopened && !outs->stream)
+        check_fails = TRUE;
+      if(!outs->fopened && outs->stream)
+        check_fails = TRUE;
+      if(!outs->fopened && outs->bytes)
+        check_fails = TRUE;
+    }
+    else {
+      /* standard stream */
+      if(!outs->stream || outs->s_isreg || outs->fopened)
+        check_fails = TRUE;
+      if(outs->alloc_filename || outs->init)
+        check_fails = TRUE;
+    }
+    if(check_fails) {
+      warnf(config, "Invalid output struct data for write callback\n");
+      return failure;
+    }
   }
 #endif
 
-  if(!out->stream) {
+  if(!outs->stream) {
-    out->bytes = 0; /* nothing written yet */
+    FILE *file;
-    if(!out->filename) {
+
+    if(!outs->filename || !*outs->filename) {
       warnf(config, "Remote filename has no length!\n");
-      return err_rc; /* Failure */
+      return failure;
     }
 
     if(config->content_disposition) {
       /* don't overwrite existing files */
-      FILE* f = fopen(out->filename, "r");
+      file = fopen(outs->filename, "rb");
-      if(f) {
+      if(file) {
-        fclose(f);
+        fclose(file);
-        warnf(config, "Refusing to overwrite %s: %s\n", out->filename,
+        warnf(config, "Refusing to overwrite %s: %s\n", outs->filename,
               strerror(EEXIST));
-        return err_rc; /* Failure */
+        return failure;
       }
     }
 
     /* open file for writing */
-    out->stream = fopen(out->filename, "wb");
+    file = fopen(outs->filename, "wb");
-    if(!out->stream) {
+    if(!file) {
-      warnf(config, "Failed to create the file %s: %s\n", out->filename,
+      warnf(config, "Failed to create the file %s: %s\n", outs->filename,
             strerror(errno));
-      return err_rc; /* failure */
+      return failure;
     }
+    outs->s_isreg = TRUE;
+    outs->fopened = TRUE;
+    outs->stream = file;
+    outs->bytes = 0;
+    outs->init = 0;
   }
 
-  rc = fwrite(buffer, sz, nmemb, out->stream);
+  rc = fwrite(buffer, sz, nmemb, outs->stream);
 
   if((sz * nmemb) == rc)
     /* we added this amount of data to the output */
-    out->bytes += (sz * nmemb);
+    outs->bytes += (sz * nmemb);
 
   if(config->readbusy) {
     config->readbusy = FALSE;
@@ -97,12 +135,10 @@ size_t tool_write_cb(void *buffer, size_t sz, size_t nmemb, void *userdata)
   }
 
   if(config->nobuffer) {
-    /* disable output buffering */
+    /* output buffering disabled */
-    int res = fflush(out->stream);
+    int res = fflush(outs->stream);
-    if(res) {
+    if(res)
-      /* return a value that isn't the same as sz * nmemb */
+      return failure;
-      return err_rc; /* failure */
-    }
   }
 
   return rc;

src/tool_cfgable.c

@@ -127,7 +127,5 @@ void free_config_fields(struct Configurable *config)
   Curl_safefree(config->ftp_alternative_to_user);
 
   Curl_safefree(config->libcurl);
-
-  config->outs = NULL; /* closed elsewhere when appropriate */
 }
 

src/tool_cfgable.h

@@ -190,7 +190,7 @@ struct Configurable {
 
   int default_node_flags;   /* default flags to search for each 'node', which
                                is basically each given URL to transfer */
-  struct OutStruct *outs;
+
   bool xattr;               /* store metadata in extended attributes */
   long gssapi_delegation;
 

src/tool_sdecls.h

@@ -26,27 +26,43 @@
 
 /*
  * OutStruct variables keep track of information relative to curl's
- * output writing, which may take place to stdout or to some file.
+ * output writing, which may take place to a standard stream or a file.
  *
- * 'filename' member is a pointer to either a file name string or to
+ * 'filename' member is either a pointer to a file name string or NULL
- * string "-" to indicate that output is written to stdout.
+ * when dealing with a standard stream.
  *
  * 'alloc_filename' member is TRUE when string pointed by 'filename' has been
  * dynamically allocated and 'belongs' to this OutStruct, otherwise FALSE.
  *
- * 'stream' member is a pointer to a stream controlling object as returned
+ * 's_isreg' member is TRUE when output goes to a regular file, this also
- * from a 'fopen' call or stdout. When 'stdout' this shall not be closed.
+ * implies that output is 'seekable' and 'appendable' and also that member
+ * 'filename' points to file name's string. For any standard stream member
+ * 's_isreg' will be FALSE.
  *
- * 'bytes' member represents amount written, and 'init' initial file size.
+ * 'fopened' member is TRUE when output goes to a regular file and it
+ * has been fopen'ed, requiring it to be closed later on. In any other
+ * case this is FALSE.
+ *
+ * 'stream' member is a pointer to a stream controlling object as returned
+ * from a 'fopen' call or a standard stream.
+ *
+ * 'config' member is a pointer to associated 'Configurable' struct.
+ *
+ * 'bytes' member represents amount written so far.
+ *
+ * 'init' member holds original file size or offset at which truncation is
+ * taking place. Always zero unless appending to a non-empty regular file.
  */
 
 struct OutStruct {
-  char *filename;               /* pointer to file name or "-" string */
+  char *filename;
-  bool alloc_filename;          /* allocated filename belongs to this */
+  bool alloc_filename;
-  FILE *stream;                 /* stdout or stream controlling object */
+  bool s_isreg;
-  struct Configurable *config;  /* pointer back to Configurable struct */
+  bool fopened;
-  curl_off_t bytes;             /* amount written so far */
+  FILE *stream;
-  curl_off_t init;              /* original size (non-zero when appending) */
+  struct Configurable *config;
+  curl_off_t bytes;
+  curl_off_t init;
 };