Merge r409942, r607245, r607440 from trunk:

* Remove temporary files if renaming fails, otherwise they may accumulate.

* Make loglevel consistent for similar situations. As this really should not
  happen set it to error.

* Change loglevel from ERROR to WARNING if the renaming of the vary, headers
  or data file fails.

Submitted by: Davi Arnaut <davi haxent.com.br>
Reviewed by: rpluem, niq, wrowe

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@607443 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index e98515cb435dad83c8f89c91b413e706b43a0911..a3defcf2e465ac2e3e29014762c8d10844eea1a7 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -10,6 +10,9 @@ Changes with Apache 2.2.7
      mod_imagemap: Fix a cross-site scripting issue.  Reported by JPCERT.
      [Joe Orton]
 
+  *) mod_disk_cache: Delete temporary files if they cannot be renamed to their
+     final name. [Davi Arnaut <davi haxent.com.br>]
+
   *) http_protocol: Escape request method in 405 error reporting.
      This has no security impact since the browser cannot be tricked
      into sending arbitrary method strings.  [Jeff Trawick]

diff --git a/STATUS b/STATUS
index 57660100d063247a29b33ea17822f3d55e77f883..2199283f04e55cac7dc649805e23e615678f7b81 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -122,25 +122,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_disk_cache: Delete temporary files if they cannot be renamed to their
-     final name.
-      Trunk version of patch:
-         http://svn.apache.org/viewcvs.cgi?rev=409942&view=rev
-         http://svn.apache.org/viewcvs.cgi?rev=607245&view=rev
-         http://svn.apache.org/viewcvs.cgi?rev=607440&view=rev
-      Backport version for 2.2.x of patch:
-         Trunk version of patch works
-      +1: rpluem, niq, wrowe
-      niq: Provisional +1, but the error logging should be at a consistent
-           level (maybe WARNING?)
-      rpluem: Set it to ERROR in all cases as IMHO this should not happen.
-              If this level is too high we can reduce it later.
-      wrowe: disagree with rpluem - it's incredibly disruptive to admins
-             to have their logs filled with noise - warning would be ok, 
-             provided there's no more than one entry per failed request.
-             If their request would die outright, only then is rpluem right.
-      rpluem: Level now set to WARNING.
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
 

diff --git a/modules/cache/mod_disk_cache.c b/modules/cache/mod_disk_cache.c
index 6b3e7733afc37bdff266c8e4cf62ea550017e146..4148b373ca2aaa7fb7842958a9636e11dea9ab52 100644 (file)
--- a/modules/cache/mod_disk_cache.c
+++ b/modules/cache/mod_disk_cache.c
@@ -165,7 +165,10 @@ static apr_status_t file_cache_el_final(disk_cache_object_t *dobj,
          */
         rv = apr_file_rename(dobj->tempfile, dobj->datafile, r->pool);
         if (rv != APR_SUCCESS) {
-            /* XXX log */
+            ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+                         "disk_cache: rename tempfile to datafile failed:"
+                         " %s -> %s", dobj->tempfile, dobj->datafile);
+            apr_file_remove(dobj->tempfile, r->pool);
         }
 
         dobj->tfd = NULL;
@@ -856,9 +859,10 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, cache_info
             rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile,
                                   r->pool);
             if (rv != APR_SUCCESS) {
-                ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
+                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
                     "disk_cache: rename tempfile to varyfile failed: %s -> %s",
                     dobj->tempfile, dobj->hdrsfile);
+                apr_file_remove(dobj->tempfile, r->pool);
                 return rv;
             }
 
@@ -946,9 +950,10 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, cache_info
 
     rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile, r->pool);
     if (rv != APR_SUCCESS) {
-        ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server,
+        ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
                      "disk_cache: rename tempfile to hdrsfile failed: %s -> %s",
                      dobj->tempfile, dobj->hdrsfile);
+        apr_file_remove(dobj->tempfile, r->pool);
         return rv;
     }