Re: svn commit: r1878996 - /httpd/httpd/branches/2.4.x/STATUS

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1878996 - /httpd/httpd/branches/2.4.x/STATUS

Yann Ylavic
On Fri, Jun 19, 2020 at 6:05 PM <[hidden email]> wrote:

>
> Author: ylavic
> Date: Fri Jun 19 16:05:56 2020
> New Revision: 1878996
>
> URL: http://svn.apache.org/viewvc?rev=1878996&view=rev
> Log:
> Re-propose ap_proxy_define_match_worker() for backport.
>
> Re new BZ 64537 (and old 43513). There's the dns/connection reuse
> compat issue spotted by covener still, to be discussed..
>
[]

> +  *) mod_proxy: Add ap_proxy_define_match_worker() and use it for ProxyPassMatch
> +     and ProxyMatch section to distinguish between normal workers and workers
> +     with regex substitutions in the name. Implement handling of such workers
> +     in ap_proxy_get_worker(). Fixes the bug when regex workers were not
> +     matched and used for request. PR 43513 and 64537.
> +     trunk patch: http://svn.apache.org/r1609680
> +                  http://svn.apache.org/r1609688
> +                  http://svn.apache.org/r1641381
> +                  http://svn.apache.org/r1826289
> +                  http://svn.apache.org/r1826313
> +                  http://svn.apache.org/r1878467
> +                  http://svn.apache.org/r1878994
> +     2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker-v4.patch
> +     +1: ylavic
> +     -0: covener: lots of folks getting dns and connection reuse implicitly after we backport this.
> +     ylavic: reviving this, should we have an opt-in for 2.4.x?
> +             (eg. an explicit enablereuse=on)

Couldn't we apply something like this for 2.4.x compat?
If so, 2.4.x only or trunk/2.5/2.6/3.0 too?

Index: modules/proxy/proxy_util.c
===================================================================
--- modules/proxy/proxy_util.c    (revision 1878994)
+++ modules/proxy/proxy_util.c    (working copy)
@@ -1962,6 +1962,15 @@ PROXY_DECLARE(char *) ap_proxy_define_match_worker
     }

     (*worker)->s->is_name_matchable = 1;
+    if (pdollar) {
+        /* Prior to 2.4.4x, regex workers with dollar substitution
+         * were never matched and fell into the generic worker. To
+         * avoid dns/connection reuse compat issues, let's set
+         * disablereuse by default, which can still be overwritten
+         * by an explicit enablereuse=on.
+         */
+        (*worker)->s->disablereuse = 1;
+    }
     return NULL;
 }

--

Regards;
Yann.
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1878996 - /httpd/httpd/branches/2.4.x/STATUS

Eric Covener
On Fri, Jun 19, 2020 at 12:33 PM Yann Ylavic <[hidden email]> wrote:

>
> On Fri, Jun 19, 2020 at 6:05 PM <[hidden email]> wrote:
> >
> > Author: ylavic
> > Date: Fri Jun 19 16:05:56 2020
> > New Revision: 1878996
> >
> > URL: http://svn.apache.org/viewvc?rev=1878996&view=rev
> > Log:
> > Re-propose ap_proxy_define_match_worker() for backport.
> >
> > Re new BZ 64537 (and old 43513). There's the dns/connection reuse
> > compat issue spotted by covener still, to be discussed..
> >
> []
> > +  *) mod_proxy: Add ap_proxy_define_match_worker() and use it for ProxyPassMatch
> > +     and ProxyMatch section to distinguish between normal workers and workers
> > +     with regex substitutions in the name. Implement handling of such workers
> > +     in ap_proxy_get_worker(). Fixes the bug when regex workers were not
> > +     matched and used for request. PR 43513 and 64537.
> > +     trunk patch: http://svn.apache.org/r1609680
> > +                  http://svn.apache.org/r1609688
> > +                  http://svn.apache.org/r1641381
> > +                  http://svn.apache.org/r1826289
> > +                  http://svn.apache.org/r1826313
> > +                  http://svn.apache.org/r1878467
> > +                  http://svn.apache.org/r1878994
> > +     2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-ap_proxy_define_match_worker-v4.patch
> > +     +1: ylavic
> > +     -0: covener: lots of folks getting dns and connection reuse implicitly after we backport this.
> > +     ylavic: reviving this, should we have an opt-in for 2.4.x?
> > +             (eg. an explicit enablereuse=on)
>
> Couldn't we apply something like this for 2.4.x compat?
> If so, 2.4.x only or trunk/2.5/2.6/3.0 too?
>
> Index: modules/proxy/proxy_util.c
> ===================================================================
> --- modules/proxy/proxy_util.c    (revision 1878994)
> +++ modules/proxy/proxy_util.c    (working copy)
> @@ -1962,6 +1962,15 @@ PROXY_DECLARE(char *) ap_proxy_define_match_worker
>      }
>
>      (*worker)->s->is_name_matchable = 1;
> +    if (pdollar) {
> +        /* Prior to 2.4.4x, regex workers with dollar substitution
> +         * were never matched and fell into the generic worker. To
> +         * avoid dns/connection reuse compat issues, let's set
> +         * disablereuse by default, which can still be overwritten
> +         * by an explicit enablereuse=on.
> +         */
> +        (*worker)->s->disablereuse = 1;
> +    }
>      return NULL;
>  }
>

+1
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1878996 - /httpd/httpd/branches/2.4.x/STATUS

Yann Ylavic
On Fri, Jun 19, 2020 at 6:54 PM Eric Covener <[hidden email]> wrote:
>
> +1

Thanks, r1879000, backport proposal updated too (though we probably
want to wait for an actual test in BZ 64537).