From 7be872c389a4cc97a4594912d1e0bdd865974780 Mon Sep 17 00:00:00 2001 From: Yang Tse Date: Fri, 30 Sep 2011 20:56:56 +0200 Subject: [PATCH] curl tool: fix some more OOM handling --- src/main.c | 134 +++++++++++++++++++++++++++-------------------------- src/tool_cb_hdr.c | 9 +++- src/tool_cb_wrt.c | 86 ++++++++++++++++++++++++---------- src/tool_cfgable.c | 2 - src/tool_cfgable.h | 2 +- src/tool_sdecls.h | 38 ++++++++++----- 6 files changed, 166 insertions(+), 105 deletions(-) diff --git a/src/main.c b/src/main.c index 05b95bb..ce3f503 100644 --- a/src/main.c +++ b/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) { - /* 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((urlnode->flags & GETOUT_USEREMOTE) + && config->content_disposition) { + /* Our header callback sets the filename */ + DEBUGASSERT(!outs.filename); } + else { + if(config->resume_from_current) { + /* 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; + } - 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; + 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.bytes = 0; /* reset byte counter */ + 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 && - outfile && !curlx_strequal(outfile, "-") && outs.stream) { + if(!res && config->xattr && outs.fopened && 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 && - outs.filename && !curlx_strequal("-", outs.filename)) { + if(!res && config->remote_time && outs.s_isreg && 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(strcmp(heads.filename, "-") && heads.stream) - fclose(heads.stream); - if(heads.alloc_filename) - Curl_safefree(heads.filename); - } + if(heads.fopened && heads.stream) + fclose(heads.stream); + if(heads.alloc_filename) + Curl_safefree(heads.filename); if(config->trace_fopened && config->trace_stream) fclose(config->trace_stream); diff --git a/src/tool_cb_hdr.c b/src/tool_cb_hdr.c index 1565786..06ced45 100644 --- a/src/tool_cb_hdr.c +++ b/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 diff --git a/src/tool_cb_wrt.c b/src/tool_cb_wrt.c index 1889080..de94f6e 100644 --- a/src/tool_cb_wrt.c +++ b/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 Configurable *config = out->config; + struct OutStruct *outs = userdata; + 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) { - out->bytes = 0; /* nothing written yet */ - if(!out->filename) { + if(!outs->stream) { + FILE *file; + + 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"); - if(f) { - fclose(f); - warnf(config, "Refusing to overwrite %s: %s\n", out->filename, + file = fopen(outs->filename, "rb"); + if(file) { + fclose(file); + 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"); - if(!out->stream) { - warnf(config, "Failed to create the file %s: %s\n", out->filename, + file = fopen(outs->filename, "wb"); + if(!file) { + 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 */ - int res = fflush(out->stream); - if(res) { - /* return a value that isn't the same as sz * nmemb */ - return err_rc; /* failure */ - } + /* output buffering disabled */ + int res = fflush(outs->stream); + if(res) + return failure; } return rc; diff --git a/src/tool_cfgable.c b/src/tool_cfgable.c index 8059fa0..e74ea43 100644 --- a/src/tool_cfgable.c +++ b/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 */ } diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h index c324be9..f27a8e2 100644 --- a/src/tool_cfgable.h +++ b/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; diff --git a/src/tool_sdecls.h b/src/tool_sdecls.h index 08203a1..5036527 100644 --- a/src/tool_sdecls.h +++ b/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 - * string "-" to indicate that output is written to stdout. + * 'filename' member is either a pointer to a file name string or NULL + * 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. * + * 's_isreg' member is TRUE when output goes to a regular file, this also + * 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. + * + * '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 stdout. When 'stdout' this shall not be closed. + * from a 'fopen' call or a standard stream. + * + * 'config' member is a pointer to associated 'Configurable' struct. + * + * 'bytes' member represents amount written so far. * - * 'bytes' member represents amount written, and 'init' initial file size. + * '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 */ - bool alloc_filename; /* allocated filename belongs to this */ - FILE *stream; /* stdout or stream controlling object */ - struct Configurable *config; /* pointer back to Configurable struct */ - curl_off_t bytes; /* amount written so far */ - curl_off_t init; /* original size (non-zero when appending) */ + char *filename; + bool alloc_filename; + bool s_isreg; + bool fopened; + FILE *stream; + struct Configurable *config; + curl_off_t bytes; + curl_off_t init; }; -- 2.7.4