On Thu, Jan 7, 2021 at 5:10 PM Eric Covener <cove...@gmail.com> wrote:
>
> On Thu, Jan 7, 2021 at 11:00 AM Yann Ylavic <ylavic....@gmail.com> wrote:
> >
> > On Thu, Jan 7, 2021 at 2:19 PM <yla...@apache.org> wrote:
> > >
> > > Author: ylavic
> > > Date: Thu Jan  7 13:19:08 2021
> > > New Revision: 1885239
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1885239&view=rev
> > > Log:
> > > mod_proxy_wstunnel: leave Upgrade requests handling to mod_proxy_http.
> > [snip]
> > > +static int proxy_wstunnel_post_config(apr_pool_t *pconf, apr_pool_t 
> > > *plog,
> > > +                                      apr_pool_t *ptemp, server_rec *s)
> > > +{
> > > +    fallback_to_mod_proxy_http = 0;
> > > +    if (ap_state_query(AP_SQ_MAIN_STATE) != AP_SQ_MS_CREATE_PRE_CONFIG) {
> > > +        apr_size_t i = 0;
> > > +        const module *mod;
> > > +        while ((mod = ap_loaded_modules[i++])) {
> > > +            if (strcmp(mod->name, "mod_proxy_http.c") == 0) {
> > > +                fallback_to_mod_proxy_http = 1;
> > > +                break;
> > > +            }
> > > +        }
> > > +    }
> >
>
> doesn't ap_find_linked_module() do this?

I tried to find it by grep-ing and following "ap_loaded_modules" and
was a bit surprised it didn't exist actually. I should have looked for
the other "ap_top_module" list :)
Changed in r1885244.

>
> > I wonder if, instead of the above loop to check whether mod_proxy_http
> > is loaded, we'd better have an OPTIONAL_FN registered by
> > mod_proxy_http and retrieved here.
>
> > ISTR that we allow for different versions of (proxy) modules to run
> > together, so if a user upgrades to the latest mod_proxy_wstunnel but
> > keeps an older mod_proxy_http the above would not work..
>
> I don't think we should have to tolerate this, not for included
> modules. Is the context maybe for downstream distributions?

I was thinking of them yes, but I suppose that the maintainers also
apply fixes with their whole context, and can figure out here that
both modules need an update (if they ever want to apply this).

>
> > Maybe I should apply the attached patch instead, or is it overkill?
>
> since it's already written and short and conceptually pretty simple I
> think it's good.

OK, just added as is to the backport proposal.

Thanks Eric.

Reply via email to