Hello! On Mon, Jul 07, 2014 at 05:19:45PM +0800, flygoast wrote:
> # HG changeset patch > # User FengGu <[email protected]> > # Date 1404723967 -28800 > # Mon Jul 07 17:06:07 2014 +0800 > # Node ID d1d597fbf6d8e08059a308d8900e5f90def5377a > # Parent a680bf4dddd5c4b106419e3dfb0264815c401275 > Upstream: delete temp file when error in ngx_http_upstream_store(). > > > Previously, didn't process the case when ngx_http_map_uri_to_path() returned > NULL. And when error occured, didn't delete the temp file. > > > diff -r a680bf4dddd5 -r d1d597fbf6d8 src/http/ngx_http_upstream.c > --- a/src/http/ngx_http_upstream.cSat Jul 05 23:29:47 2014 +0400 > +++ b/src/http/ngx_http_upstream.cMon Jul 07 17:06:07 2014 +0800 > @@ -3375,14 +3375,16 @@ > > if (u->conf->store_lengths == NULL) { > > - ngx_http_map_uri_to_path(r, &path, &root, 0); > + if (ngx_http_map_uri_to_path(r, &path, &root, 0) == NULL) { > + goto error; > + } > > } else { > if (ngx_http_script_run(r, &path, u->conf->store_lengths->elts, 0, > u->conf->store_values->elts) > == NULL) > { > - return; > + goto error; > } > } > > @@ -3393,6 +3395,16 @@ > tf->file.name.data, path.data); > > (void) ngx_ext_rename_file(&tf->file.name, &path, &ext); > + > + return; > + > +error: > + > + if (ngx_delete_file(tf->file.name.data) == NGX_FILE_ERROR) { > + ngx_log_error(NGX_LOG_CRIT, r->connection->log, ngx_errno, > + ngx_delete_file_n " \"%s\" failed", > + tf->file.name.data); > + } > } > Thanks for noting this. What do you think about the following patch instead? There is a code in ngx_http_upstream_finalize_request() which normally removes temporary files after errors with proxy_store, but in this particular case it won't be triggered due to u->store set to 0 even in case of errors. With this patch, u->store is set to 0 only in if there are no errors, and thus temporary files are properly removed in case of errors in ngx_http_upstream_store(). diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -3253,7 +3253,6 @@ ngx_http_upstream_process_request(ngx_ht || u->headers_in.content_length_n == tf->offset)) { ngx_http_upstream_store(r, u); - u->store = 0; } } } @@ -3375,7 +3374,9 @@ ngx_http_upstream_store(ngx_http_request if (u->conf->store_lengths == NULL) { - ngx_http_map_uri_to_path(r, &path, &root, 0); + if (ngx_http_map_uri_to_path(r, &path, &root, 0) == NULL) { + return; + } } else { if (ngx_http_script_run(r, &path, u->conf->store_lengths->elts, 0, @@ -3392,7 +3393,11 @@ ngx_http_upstream_store(ngx_http_request "upstream stores \"%s\" to \"%s\"", tf->file.name.data, path.data); - (void) ngx_ext_rename_file(&tf->file.name, &path, &ext); + if (ngx_ext_rename_file(&tf->file.name, &path, &ext) != NGX_OK) { + return; + } + + u->store = 0; } -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
