Hi Yann,

Am 12.03.2018 um 13:24 schrieb yla...@apache.org:
Author: ylavic
Date: Mon Mar 12 12:24:27 2018
New Revision: 1826543

URL: http://svn.apache.org/viewvc?rev=1826543&view=rev
Log:
Fix timeout logging in ap_process_request().

We can't use 'r' after ap_process_request_after_handler(), the core output
filter might have cleaned up its deferred bucket brigade on error, including
the EOR bucket.

Reported by: steffenal
Fixes SpiderLabs/ModSecurity#1542


Modified:
     httpd/httpd/branches/2.4.x/modules/http/http_request.c

Modified: httpd/httpd/branches/2.4.x/modules/http/http_request.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/http/http_request.c?rev=1826543&r1=1826542&r2=1826543&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/http/http_request.c (original)
+++ httpd/httpd/branches/2.4.x/modules/http/http_request.c Mon Mar 12 12:24:27 
2018
@@ -480,13 +480,9 @@ AP_DECLARE(void) ap_process_request(requ
               * Notice a timeout as an error message. This might be
               * valuable for detecting clients with broken network
               * connections or possible DoS attacks.
-             *
-             * It is still safe to use r / r->pool here as the eor bucket
-             * could not have been destroyed in the event of a timeout.
               */
-            ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, r, APLOGNO(01581)
-                          "Timeout while writing data for URI %s to the"
-                          " client", r->unparsed_uri);
+            ap_log_cerror(APLOG_MARK, APLOG_INFO, rv, c, APLOGNO(01581)
+                          "flushing data to the client");
          }
      }
      if (ap_extended_status) {

was it intentional to apply this directly to 2.4 without trunk (don't know whether it applies there) and STATUS? Maybe because it is a logging only change?

And was it intentional to change the contents of the log message, not only cerror/c instead of rerror/r? If the change in log message text was intentional, shouldn't we then use a new APLOGNO? And while "Timeout" sounds like INFO or higher, "flushing" sounds more like a DEBUG or lower, at least to me.

Regards,

Rainer

Reply via email to