You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2011/01/05 01:23:43 UTC

svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Author: minfrin
Date: Wed Jan  5 00:23:43 2011
New Revision: 1055250

URL: http://svn.apache.org/viewvc?rev=1055250&view=rev
Log:
mod_proxy_http: Allocate the fake backend request from a child pool
of the backend connection, instead of misusing the pool of the frontend
request. Fixes a thread safety issue where buckets set aside in the
backend connection leak into other threads, and then disappear when
the frontend request is cleaned up, in turn causing corrupted buckets
to make other threads spin.

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
    httpd/httpd/trunk/modules/proxy/proxy_util.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1055250&r1=1055249&r2=1055250&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Jan  5 00:23:43 2011
@@ -2,6 +2,13 @@
 
 Changes with Apache 2.3.11
 
+  *) mod_proxy_http: Allocate the fake backend request from a child pool
+     of the backend connection, instead of misusing the pool of the frontend
+     request. Fixes a thread safety issue where buckets set aside in the
+     backend connection leak into other threads, and then disappear when
+     the frontend request is cleaned up, in turn causing corrupted buckets
+     to make other threads spin. [Graham Leggett]
+
   *) mod_ssl: Change the format of the SSL_{CLIENT,SERVER}_{I,S}_DN variables
      to be RFC 2253 compatible, convert non-ASCII characters to UTF8, and 
      escape other special characters with backslashes. The old format can

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1055250&r1=1055249&r2=1055250&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Wed Jan  5 00:23:43 2011
@@ -1397,7 +1397,6 @@ apr_status_t ap_proxy_http_process_respo
     char buffer[HUGE_STRING_LEN];
     const char *buf;
     char keepchar;
-    request_rec *rp;
     apr_bucket *e;
     apr_bucket_brigade *bb, *tmp_bb;
     apr_bucket_brigade *pass_bb;
@@ -1448,21 +1447,21 @@ apr_status_t ap_proxy_http_process_respo
      * filter chain
      */
 
-    rp = ap_proxy_make_fake_req(origin, r);
+    backend->r = ap_proxy_make_fake_req(origin, r);
     /* In case anyone needs to know, this is a fake request that is really a
      * response.
      */
-    rp->proxyreq = PROXYREQ_RESPONSE;
+    backend->r->proxyreq = PROXYREQ_RESPONSE;
     tmp_bb = apr_brigade_create(p, c->bucket_alloc);
     do {
         apr_status_t rc;
 
         apr_brigade_cleanup(bb);
 
-        rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), rp, 0, &len);
+        rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), backend->r, 0, &len);
         if (len == 0) {
             /* handle one potential stray CRLF */
-            rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), rp, 0, &len);
+            rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), backend->r, 0, &len);
         }
         if (len <= 0) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r,
@@ -1590,7 +1589,7 @@ apr_status_t ap_proxy_http_process_respo
                          "Set-Cookie", NULL);
 
             /* shove the headers direct into r->headers_out */
-            ap_proxy_read_headers(r, rp, buffer, sizeof(buffer), origin,
+            ap_proxy_read_headers(r, backend->r, buffer, sizeof(buffer), origin,
                                   &pread_len);
 
             if (r->headers_out == NULL) {
@@ -1656,7 +1655,7 @@ apr_status_t ap_proxy_http_process_respo
                 ap_set_content_type(r, apr_pstrdup(p, buf));
             }
             if (!ap_is_HTTP_INFO(proxy_status)) {
-                ap_proxy_pre_http_request(origin, rp);
+                ap_proxy_pre_http_request(origin, backend->r);
             }
 
             /* Clear hop-by-hop headers */
@@ -1798,7 +1797,7 @@ apr_status_t ap_proxy_http_process_respo
             if (!r->header_only && /* not HEAD request */
                 (proxy_status != HTTP_NO_CONTENT) && /* not 204 */
                 (proxy_status != HTTP_NOT_MODIFIED)) { /* not 304 */
-                ap_discard_request_body(rp);
+                ap_discard_request_body(backend->r);
             }
             return proxy_status;
         }
@@ -1814,14 +1813,14 @@ apr_status_t ap_proxy_http_process_respo
              * TE, so that they are preserved accordingly for
              * ap_http_filter to know where to end.
              */
-            rp->headers_in = apr_table_copy(r->pool, r->headers_out);
+            backend->r->headers_in = apr_table_copy(backend->r->pool, r->headers_out);
             /*
              * Restore Transfer-Encoding header from response if we saved
              * one before and there is none left. We need it for the
              * ap_http_filter. See above.
              */
-            if (te && !apr_table_get(rp->headers_in, "Transfer-Encoding")) {
-                apr_table_add(rp->headers_in, "Transfer-Encoding", te);
+            if (te && !apr_table_get(backend->r->headers_in, "Transfer-Encoding")) {
+                apr_table_add(backend->r->headers_in, "Transfer-Encoding", te);
             }
 
             apr_table_unset(r->headers_out,"Transfer-Encoding");
@@ -1853,7 +1852,7 @@ apr_status_t ap_proxy_http_process_respo
                     apr_off_t readbytes;
                     apr_status_t rv;
 
-                    rv = ap_get_brigade(rp->input_filters, bb,
+                    rv = ap_get_brigade(backend->r->input_filters, bb,
                                         AP_MODE_READBYTES, mode,
                                         conf->io_buffer_size);
 

Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1055250&r1=1055249&r2=1055250&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
+++ httpd/httpd/trunk/modules/proxy/proxy_util.c Wed Jan  5 00:23:43 2011
@@ -348,16 +348,20 @@ PROXY_DECLARE(const char *)
 
 PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
 {
-    request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
+    apr_pool_t *pool;
+
+    apr_pool_create(&pool, c->pool);
+
+    request_rec *rp = apr_pcalloc(pool, sizeof(*r));
 
-    rp->pool            = r->pool;
+    rp->pool            = pool;
     rp->status          = HTTP_OK;
 
-    rp->headers_in      = apr_table_make(r->pool, 50);
-    rp->subprocess_env  = apr_table_make(r->pool, 50);
-    rp->headers_out     = apr_table_make(r->pool, 12);
-    rp->err_headers_out = apr_table_make(r->pool, 5);
-    rp->notes           = apr_table_make(r->pool, 5);
+    rp->headers_in      = apr_table_make(pool, 50);
+    rp->subprocess_env  = apr_table_make(pool, 50);
+    rp->headers_out     = apr_table_make(pool, 12);
+    rp->err_headers_out = apr_table_make(pool, 5);
+    rp->notes           = apr_table_make(pool, 5);
 
     rp->server = r->server;
     rp->proxyreq = r->proxyreq;
@@ -368,7 +372,7 @@ PROXY_DECLARE(request_rec *)ap_proxy_mak
     rp->proto_output_filters  = c->output_filters;
     rp->proto_input_filters   = c->input_filters;
 
-    rp->request_config  = ap_create_request_config(r->pool);
+    rp->request_config  = ap_create_request_config(pool);
     proxy_run_create_req(r, rp);
 
     return rp;
@@ -1724,6 +1728,11 @@ static apr_status_t connection_cleanup(v
         return APR_SUCCESS;
     }
 
+    if (conn->r) {
+        apr_pool_destroy(conn->r->pool);
+        conn->r = NULL;
+    }
+
 #if APR_HAS_THREADS
     /* Sanity check: Did we already return the pooled connection? */
     if (conn->inreslist) {



RE: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
 

> -----Original Message-----
> From: Graham Leggett 
> Sent: Mittwoch, 5. Januar 2011 10:30
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1055250 - in /httpd/httpd/trunk: 
> CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
> 
> On 05 Jan 2011, at 9:43 AM, Ruediger Pluem wrote:

> >>             /*
> >>              * Restore Transfer-Encoding header from 
> response if we  
> >> saved
> >>              * one before and there is none left. We need 
> it for the
> >>              * ap_http_filter. See above.
> >>              */
> >> -            if (te && !apr_table_get(rp->headers_in, "Transfer- 
> >> Encoding")) {
> >> -                apr_table_add(rp->headers_in, 
> "Transfer-Encoding",  
> >> te);
> >> +            if (te && !apr_table_get(backend->r->headers_in,  
> >> "Transfer-Encoding")) {
> >> +                apr_table_add(backend->r->headers_in, "Transfer- 
> >> Encoding", te);
> >
> > Doesn't that mean that we will get entries in backend->r->headers  
> > that have been allocated
> > from r->pool instead of backend->r->pool? te is an element from r- 
> > >headers_out.
> 
> In this case we shouldn't, as apr_table_add() makes a copy of 
> te from  
> backend->r->pool. If we'd used apr_table_addn(), then it would have  
> been yes.

Good catch. I missed that.

Regards

Rüdiger


Re: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Graham Leggett <mi...@sharp.fm>.
On 05 Jan 2011, at 9:43 AM, Ruediger Pluem wrote:

> What about the comment in mod_proxy.h about the r element of  
> proxy_conn_rec?
>
> /* Request record of the frontend request
> * which the backend currently answers. */
>
> Doesn't this comment need to be adjusted now?

It does - I've fixed it in r1055367.

>> -            rp->headers_in = apr_table_copy(r->pool, r- 
>> >headers_out);
>> +            backend->r->headers_in = apr_table_copy(backend->r- 
>> >pool, r->headers_out);
>
> Doesn't that mean that we will get entries in backend->r->headers  
> that have been allocated
> from r->pool instead of backend->r->pool?

Hmmm... it does yes, I think we need an apr_table_clone here.

>>             /*
>>              * Restore Transfer-Encoding header from response if we  
>> saved
>>              * one before and there is none left. We need it for the
>>              * ap_http_filter. See above.
>>              */
>> -            if (te && !apr_table_get(rp->headers_in, "Transfer- 
>> Encoding")) {
>> -                apr_table_add(rp->headers_in, "Transfer-Encoding",  
>> te);
>> +            if (te && !apr_table_get(backend->r->headers_in,  
>> "Transfer-Encoding")) {
>> +                apr_table_add(backend->r->headers_in, "Transfer- 
>> Encoding", te);
>
> Doesn't that mean that we will get entries in backend->r->headers  
> that have been allocated
> from r->pool instead of backend->r->pool? te is an element from r- 
> >headers_out.

In this case we shouldn't, as apr_table_add() makes a copy of te from  
backend->r->pool. If we'd used apr_table_addn(), then it would have  
been yes.

Regards,
Graham
--


Re: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 01/05/2011 01:23 AM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Wed Jan  5 00:23:43 2011
> New Revision: 1055250
> 
> URL: http://svn.apache.org/viewvc?rev=1055250&view=rev
> Log:
> mod_proxy_http: Allocate the fake backend request from a child pool
> of the backend connection, instead of misusing the pool of the frontend
> request. Fixes a thread safety issue where buckets set aside in the
> backend connection leak into other threads, and then disappear when
> the frontend request is cleaned up, in turn causing corrupted buckets
> to make other threads spin.
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>     httpd/httpd/trunk/modules/proxy/proxy_util.c
> 

> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1055250&r1=1055249&r2=1055250&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Wed Jan  5 00:23:43 2011
> @@ -1448,21 +1447,21 @@ apr_status_t ap_proxy_http_process_respo
>       * filter chain
>       */
>  
> -    rp = ap_proxy_make_fake_req(origin, r);
> +    backend->r = ap_proxy_make_fake_req(origin, r);

What about the comment in mod_proxy.h about the r element of proxy_conn_rec?

/* Request record of the frontend request
 * which the backend currently answers. */

Doesn't this comment need to be adjusted now?


>      /* In case anyone needs to know, this is a fake request that is really a
>       * response.
>       */
> -    rp->proxyreq = PROXYREQ_RESPONSE;
> +    backend->r->proxyreq = PROXYREQ_RESPONSE;
>      tmp_bb = apr_brigade_create(p, c->bucket_alloc);
>      do {
>          apr_status_t rc;
>  
>          apr_brigade_cleanup(bb);
>  
> -        rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), rp, 0, &len);
> +        rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), backend->r, 0, &len);
>          if (len == 0) {
>              /* handle one potential stray CRLF */
> -            rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), rp, 0, &len);
> +            rc = ap_proxygetline(tmp_bb, buffer, sizeof(buffer), backend->r, 0, &len);
>          }
>          if (len <= 0) {
>              ap_log_rerror(APLOG_MARK, APLOG_ERR, rc, r,
> @@ -1814,14 +1813,14 @@ apr_status_t ap_proxy_http_process_respo
>               * TE, so that they are preserved accordingly for
>               * ap_http_filter to know where to end.
>               */
> -            rp->headers_in = apr_table_copy(r->pool, r->headers_out);
> +            backend->r->headers_in = apr_table_copy(backend->r->pool, r->headers_out);

Doesn't that mean that we will get entries in backend->r->headers that have been allocated
from r->pool instead of backend->r->pool?

>              /*
>               * Restore Transfer-Encoding header from response if we saved
>               * one before and there is none left. We need it for the
>               * ap_http_filter. See above.
>               */
> -            if (te && !apr_table_get(rp->headers_in, "Transfer-Encoding")) {
> -                apr_table_add(rp->headers_in, "Transfer-Encoding", te);
> +            if (te && !apr_table_get(backend->r->headers_in, "Transfer-Encoding")) {
> +                apr_table_add(backend->r->headers_in, "Transfer-Encoding", te);

Doesn't that mean that we will get entries in backend->r->headers that have been allocated
from r->pool instead of backend->r->pool? te is an element from r->headers_out.

>              }
>  
>              apr_table_unset(r->headers_out,"Transfer-Encoding");
> 

Regards

Rüdiger

Re: -Wdeclaration-after-statement

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 11 January 2011, Eric Covener wrote:
> On Tue, Jan 11, 2011 at 4:56 PM, Eric Covener <co...@gmail.com> 
wrote:
> >> The option is newer than the ability to compile the code. It
> >> doesn't appear in the gcc 3.3 doc, so I assume it has been
> >> introduced in gcc 3.4.
> >> 
> >> I have now commited a configure check for it. But I don't have a
> >> pre-3.4 gcc to test the "no" case.
> > 
> > Seems to work fine with gcc 3.3.3 on SLES9.
> 
> Your configure check that is, not the gcc feature itself.

Thanks for testing.

Cheers,
Stefan


Re: -Wdeclaration-after-statement

Posted by Eric Covener <co...@gmail.com>.
On Tue, Jan 11, 2011 at 4:56 PM, Eric Covener <co...@gmail.com> wrote:
>> The option is newer than the ability to compile the code. It doesn't
>> appear in the gcc 3.3 doc, so I assume it has been introduced in gcc
>> 3.4.
>>
>> I have now commited a configure check for it. But I don't have a
>> pre-3.4 gcc to test the "no" case.
>
> Seems to work fine with gcc 3.3.3 on SLES9.

Your configure check that is, not the gcc feature itself.

Re: -Wdeclaration-after-statement

Posted by Eric Covener <co...@gmail.com>.
> The option is newer than the ability to compile the code. It doesn't
> appear in the gcc 3.3 doc, so I assume it has been introduced in gcc
> 3.4.
>
> I have now commited a configure check for it. But I don't have a
> pre-3.4 gcc to test the "no" case.

Seems to work fine with gcc 3.3.3 on SLES9.

-Wdeclaration-after-statement

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 11 January 2011, Guenter Knauf wrote:
> Am 10.01.2011 21:26, schrieb Stefan Fritsch:
> > BTW, gcc has -Wdeclaration-after-statement to catch these things.
> > Should we add that to the default warnings enabled in maintainer
> > mode?
> 
> +2 - NetWare compiler cant deal with either;
> and +1 to treat it even as error rather than warning:
> -Werror=declarationafterstatement

The -Werror=... syntax requires a rather recent gcc (definitely 4.x, 
and x > 0 IIRC).

> 
> BTW. isnt the option without dashes? My 'man gcc' lists:
> Wdeclarationafterstatement (C and ObjectiveC only)
>       Warn when a declaration is found after a statement in a
> block. This construct, known from C++, was introduced with ISO C99
> and is by default allowed in GCC.  It is not supported by ISO C90
> and was not supported by GCC versions before GCC 3.0.
> 
> Since GCC < 3 didnt support this I assume that then also the above
> options are not recognized - so adding them should depend on GCC
> version

The option is newer than the ability to compile the code. It doesn't 
appear in the gcc 3.3 doc, so I assume it has been introduced in gcc 
3.4.

I have now commited a configure check for it. But I don't have a 
pre-3.4 gcc to test the "no" case.

Cheers,
Stefan

Re: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Guenter Knauf <fu...@apache.org>.
Am 10.01.2011 21:26, schrieb Stefan Fritsch:
> BTW, gcc has -Wdeclaration-after-statement to catch these things.
> Should we add that to the default warnings enabled in maintainer mode?
+2 - NetWare compiler cant deal with either;
and +1 to treat it even as error rather than warning:
-Werror=declarationafterstatement

BTW. isnt the option without dashes? My 'man gcc' lists:
Wdeclarationafterstatement (C and ObjectiveC only)
      Warn when a declaration is found after a statement in a block.
      This construct, known from C++, was introduced with ISO C99
      and is by default allowed in GCC.  It is not supported by ISO C90
      and was not supported by GCC versions before GCC 3.0.

Since GCC < 3 didnt support this I assume that then also the above 
options are not recognized - so adding them should depend on GCC version 
 >= 3.

Gün.



Re: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Thursday 06 January 2011, Plüm, Rüdiger, VF-Group wrote:
> > >  {
> > >
> > > -    request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
> > > +    apr_pool_t *pool;
> > > +
> > > +    apr_pool_create(&pool, c->pool);
> > > +
> > > +    request_rec *rp = apr_pcalloc(pool, sizeof(*r));
> > >
> > >  
> >
> > 
> >
> > MSVC idiosyncrasy, you cannot declare rp after doing 
> > something, in this 
> > case "something" is calling apr_pool_create.
> 
> Fixed in r1055771.

BTW, gcc has -Wdeclaration-after-statement to catch these things. 
Should we add that to the default warnings enabled in maintainer mode?

RE: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
 

> -----Original Message-----
> From: Gregg L. Smith 
> Sent: Donnerstag, 6. Januar 2011 02:56
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1055250 - in /httpd/httpd/trunk: 
> CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
> 
> Hello,
> 
> wrote:
> > Author: minfrin
> > Date: Wed Jan  5 00:23:43 2011
> > New Revision: 1055250
> 
> > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> > URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/p
> roxy_util.c?rev=1055250&r1=1055249&r2=1055250&view=diff
> > 
> ==============================================================
> ================
> > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Wed Jan  5 
> 00:23:43 2011
> > @@ -348,16 +348,20 @@ PROXY_DECLARE(const char *)
> >  
> >  PROXY_DECLARE(request_rec 
> *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
> >  {
> > -    request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
> > +    apr_pool_t *pool;
> > +
> > +    apr_pool_create(&pool, c->pool);
> > +
> > +    request_rec *rp = apr_pcalloc(pool, sizeof(*r));
> >  
> 
> MSVC idiosyncrasy, you cannot declare rp after doing 
> something, in this 
> case "something" is calling apr_pool_create.

Fixed in r1055771.

Regards

Rüdiger


Re: svn commit: r1055250 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c

Posted by "Gregg L. Smith" <li...@glewis.com>.
Hello,

minfrin@apache.org wrote:
> Author: minfrin
> Date: Wed Jan  5 00:23:43 2011
> New Revision: 1055250

> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1055250&r1=1055249&r2=1055250&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Wed Jan  5 00:23:43 2011
> @@ -348,16 +348,20 @@ PROXY_DECLARE(const char *)
>  
>  PROXY_DECLARE(request_rec *)ap_proxy_make_fake_req(conn_rec *c, request_rec *r)
>  {
> -    request_rec *rp = apr_pcalloc(r->pool, sizeof(*r));
> +    apr_pool_t *pool;
> +
> +    apr_pool_create(&pool, c->pool);
> +
> +    request_rec *rp = apr_pcalloc(pool, sizeof(*r));
>  

MSVC idiosyncrasy, you cannot declare rp after doing something, in this 
case "something" is calling apr_pool_create.

.\proxy_util.c(355) : error C2275: 'request_rec' : illegal use of this
type as an expression
         c:\build\nresvn\include\httpd.h(730) : see declaration of
'request_rec'
.\proxy_util.c(355) : error C2065: 'rp' : undeclared identifier

to the tune of 36 total errors.

It looks like this apr_pool_create and apr_pcalloc lines were 
specifically placed in this order so declare rp after pool.

apr_pool_t *pool;
request_rec *rp;

apr_pool_create(&pool, c->pool);

rp = apr_pcalloc(pool, sizeof(*r));

Thanks,

Gregg