Eliminate loop in ap_proxy_string_read(). Need to be able to handle

getting an empty brigade from ap_get_brigade(). Also made sure that
we always call ap_get_brigade() in readline mode.

Submitted/Reviewed by: Adam Sussman & Bill Stoddard

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

diff --git a/CHANGES b/CHANGES
index 01619f07a0df1a0366aacfcbe55c2a877c34104c..51a986b69180f9c71322d45b7c0d29e8cd671121 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,4 +1,6 @@
 Changes with Apache 2.0.30-dev
+  *) Eliminate loop in ap_proxy_string_read().
+     [Adam Sussman, Bill Stoddard]
 
   *) Provide $0..$9 results from mod_include regex parsing.
      [William Rowe]

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index c2da5657a1c18659163166c96a31039ad897a1b6..71828d70c4aabbfe5dba99ea09c728965e7764ca 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -992,15 +992,16 @@ PROXY_DECLARE(int) ap_proxy_pre_http_request(conn_rec *c, request_rec *r)
 }
 
 /* converts a series of buckets into a string 
- * BillS says this function looks essentially identical to ap_rgetline() 
- * in protocol.c. Deprecate this function and use apr_rgetline() instead?
+ * XXX: BillS says this function performs essentially the same function as 
+ * ap_rgetline() in protocol.c. Deprecate this function and use ap_rgetline() 
+ * instead? I think ap_proxy_string_read() will not work properly on non ASCII
+ * (EBCDIC) machines either.
  */
 PROXY_DECLARE(apr_status_t) ap_proxy_string_read(conn_rec *c, apr_bucket_brigade *bb,
                                                  char *buff, size_t bufflen, int *eos)
 {
     apr_bucket *e;
     apr_status_t rv;
-    apr_off_t readbytes = 0;   /* line-at-a-time */
     char *pos = buff;
     char *response;
     int found = 0;
@@ -1012,14 +1013,20 @@ PROXY_DECLARE(apr_status_t) ap_proxy_string_read(conn_rec *c, apr_bucket_brigade
 
     /* loop through each brigade */
     while (!found) {
-
+        apr_off_t zero = 0;
         /* get brigade from network one line at a time */
-        if (APR_SUCCESS != (rv = ap_get_brigade(c->input_filters, bb, AP_MODE_BLOCKING, &readbytes))) {
+        if (APR_SUCCESS != (rv = ap_get_brigade(c->input_filters, bb, 
+                                                AP_MODE_BLOCKING, 
+                                                &zero /* readline */))) {
             return rv;
         }
+        if (APR_BRIGADE_EMPTY(bb)) {
+            /* The connection aborted or timed out */
+            return APR_TIMEUP;
+        }
 
         /* loop through each bucket */
-        while (!found && !APR_BRIGADE_EMPTY(bb)) {
+        while (!found) {
             e = APR_BRIGADE_FIRST(bb);
             if (APR_BUCKET_IS_EOS(e)) {
                 *eos = 1;
@@ -1028,7 +1035,11 @@ PROXY_DECLARE(apr_status_t) ap_proxy_string_read(conn_rec *c, apr_bucket_brigade
                 if (APR_SUCCESS != apr_bucket_read(e, (const char **)&response, &len, APR_BLOCK_READ)) {
                     return rv;
                 }
-                /* is string LF terminated? */
+                /* is string LF terminated? 
+                 * XXX: This check can be made more efficient by simply checking 
+                 * if the last character in the 'response' buffer is an ASCII_LF.
+                 * See ap_rgetline() for an example.
+                 */
                 if (memchr(response, APR_ASCII_LF, len)) {
                     found = 1;
                 }