Introduce ap_debug_assert() macro, like ap_assert() but only active if

AP_DEBUG is defined.

ap_get_client_block():
. avoid some cases where we leak a temporary bucket brigade
. clean up/fix the logic to copy a brigade into the caller's buffer;
  the wrong length was used in some cases
. add an AP_DEBUG-only assertion for some assumptions made regarding
  the brigade returned by the filters

Submitted by:	partly by Greg Stein, but of course anything bad is mine

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@86583 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/include/httpd.h b/include/httpd.h
index 3c34bb7ea6dcc9e5b3f450dc8ff16c9d2e1d22ac..d24e98ae910d89b0de028737e8af86b2b68e662c 100644 (file)
--- a/include/httpd.h
+++ b/include/httpd.h
@@ -1599,6 +1599,10 @@ unsigned long ap_get_virthost_addr(char *hostname, unsigned short *port);
 
 /*
  * Redefine assert() to something more useful for an Apache...
+ *
+ * Use ap_assert() if the condition should always be checked.
+ * Use ap_debug_assert() if the condition should only be checked when AP_DEBUG
+ * is defined.
  */
 /**
  * Log an assertion to the error log
@@ -1611,6 +1615,12 @@ API_EXPORT(void) ap_log_assert(const char *szExp, const char *szFile, int nLine)
                            __attribute__((noreturn));
 #define ap_assert(exp) ((exp) ? (void)0 : ap_log_assert(#exp,__FILE__,__LINE__))
 
+#ifdef AP_DEBUG
+#define ap_debug_assert(exp) ap_assert(exp)
+#else
+#define ap_debug_assert(exp) ((void)0)
+#endif
+
 /* A set of flags which indicate places where the server should raise(SIGSTOP).
  * This is useful for debugging, because you can then attach to that process
  * with gdb and continue.  This is important in cases where one_process

diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c
index 5f4318393da6e3a684ef35855908ecd043347a5a..5a46a9b4888794229f3af92655350ba04ef66b44 100644 (file)
--- a/modules/http/http_protocol.c
+++ b/modules/http/http_protocol.c
@@ -2396,7 +2396,7 @@ API_EXPORT(long) ap_get_client_block(request_rec *r, char *buffer, int bufsiz)
     apr_status_t rv;
     apr_int32_t timeout;
     ap_bucket *b, *old;
-    ap_bucket_brigade *bb = ap_brigade_create(r->pool);
+    ap_bucket_brigade *bb;
 
     if (!r->read_chunked) {     /* Content-length read */
         const char *tempbuf;
@@ -2406,6 +2406,7 @@ API_EXPORT(long) ap_get_client_block(request_rec *r, char *buffer, int bufsiz)
         if (len_to_read == 0) {
             return 0;
         }
+        bb = ap_brigade_create(r->pool);
         do {
             if (AP_BRIGADE_EMPTY(bb)) {
                 apr_getsocketopt(r->connection->client->bsock, APR_SO_TIMEOUT, &timeout);
@@ -2416,6 +2417,7 @@ API_EXPORT(long) ap_get_client_block(request_rec *r, char *buffer, int bufsiz)
                      */
                     apr_setsocketopt(r->connection->client->bsock, APR_SO_TIMEOUT, timeout);
                     r->connection->keepalive = -1;
+                    ap_brigade_destroy(bb);
                     return -1;
                 }
                 apr_setsocketopt(r->connection->client->bsock, APR_SO_TIMEOUT, timeout);
@@ -2433,19 +2435,14 @@ API_EXPORT(long) ap_get_client_block(request_rec *r, char *buffer, int bufsiz)
         total = 0;
         do {
             rv = ap_bucket_read(b, &tempbuf, &len_read, 0);
-            if (len_to_read < b->length) { /* shouldn't happen */
-                ap_bucket_split(b, len_to_read);
-            }
-            else {
-                len_to_read = len_read;
-            }
-            
-            memcpy(buffer, tempbuf, len_to_read);
-            buffer += len_to_read;
-            
-            r->read_length += len_to_read;
-            total += len_to_read;
-            r->remaining -= len_to_read;
+            ap_debug_assert(total + len_read <= bufsiz); /* because we told the filter 
+                                                          * below us not to give us too much */
+            ap_debug_assert(r->remaining >= len_read);
+            memcpy(buffer, tempbuf, len_read);
+            buffer += len_read;
+            r->read_length += len_read;
+            total += len_read;
+            r->remaining -= len_read;
             old = b;
             b = AP_BUCKET_NEXT(b);
             AP_BUCKET_REMOVE(old);
@@ -2454,7 +2451,7 @@ API_EXPORT(long) ap_get_client_block(request_rec *r, char *buffer, int bufsiz)
         ap_brigade_destroy(bb);
         return total;
     }
-   
+
     /*
      * Handle chunked reading Note: we are careful to shorten the input
      * bufsiz so that there will always be enough space for us to add a CRLF