You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2021/05/12 14:35:08 UTC
Re: svn commit: r1889792 - in /httpd/httpd/trunk:
docs/manual/mod/mod_proxy_wstunnel.xml modules/proxy/mod_proxy_wstunnel.c
On 5/12/21 12:10 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Wed May 12 10:10:36 2021
> New Revision: 1889792
>
> URL: http://svn.apache.org/viewvc?rev=1889792&view=rev
> Log:
> mod_proxy_wstunnel: Add ProxyWebsocketFallbackToProxyHttp.
>
> Allows to opt-out the fallback to mod_proxy_http to handle WebSocket upgrade,
> and let mod_proxy_wstunnel handle the requests as in 2.4.46 and earlier.
>
> Update docs.
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/mod_proxy_wstunnel.xml
> httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c?rev=1889792&r1=1889791&r2=1889792&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Wed May 12 10:10:36 2021
> @@ -449,10 +476,18 @@ static const char * proxyws_set_asynch_d
> return NULL;
> }
>
> +static const char * proxyws_fallback_to_proxy_http(cmd_parms *cmd, void *conf, int arg)
> +{
> + proxyws_dir_conf *dconf = conf;
> + dconf->fallback_to_proxy_http = !!arg;
Why !!? Shouldn't just arg be enough?
> + dconf->fallback_to_proxy_http_set = 1;
> + return NULL;
> +}
> +
> 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 =
> + can_fallback_to_proxy_http =
> (ap_find_linked_module("mod_proxy_http.c") != NULL);
>
> return OK;
> @@ -467,6 +507,7 @@ static const command_rec ws_proxy_cmds[]
> AP_INIT_TAKE1("ProxyWebsocketAsyncDelay", proxyws_set_asynch_delay, NULL,
> RSRC_CONF|ACCESS_CONF,
> "amount of time to poll before going asynchronous"),
> +
Stray empty line?
> {NULL}
> };
>
Regards
Rüdiger
Re: svn commit: r1889792 - in /httpd/httpd/trunk:
docs/manual/mod/mod_proxy_wstunnel.xml modules/proxy/mod_proxy_wstunnel.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 5/12/21 5:36 PM, Yann Ylavic wrote:
> On Wed, May 12, 2021 at 4:35 PM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 5/12/21 12:10 PM, ylavic@apache.org wrote:
>>>
>>> +static const char * proxyws_fallback_to_proxy_http(cmd_parms *cmd, void *conf, int arg)
>>> +{
>>> + proxyws_dir_conf *dconf = conf;
>>> + dconf->fallback_to_proxy_http = !!arg;
>>
>> Why !!? Shouldn't just arg be enough?
>
> That's because fallback_to_proxy_http is a bit (i.e. unsigned int :1),
> and !!arg is guaranteed to be 0 or 1 (I could have used arg != 0 too).
> For instance dconf->fallback_to_proxy_http = 2 wouldn't work as
> expected, maybe arg is always 0 or 1 but some compilers warn with
> -Woverflow here anyway.
Thanks for explaining. Maybe a short comment in the code about this would save some head scratching for future code readers :-)
Regards
Rüdiger
Re: svn commit: r1889792 - in /httpd/httpd/trunk: docs/manual/mod/mod_proxy_wstunnel.xml
modules/proxy/mod_proxy_wstunnel.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, May 12, 2021 at 4:35 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 5/12/21 12:10 PM, ylavic@apache.org wrote:
> >
> > +static const char * proxyws_fallback_to_proxy_http(cmd_parms *cmd, void *conf, int arg)
> > +{
> > + proxyws_dir_conf *dconf = conf;
> > + dconf->fallback_to_proxy_http = !!arg;
>
> Why !!? Shouldn't just arg be enough?
That's because fallback_to_proxy_http is a bit (i.e. unsigned int :1),
and !!arg is guaranteed to be 0 or 1 (I could have used arg != 0 too).
For instance dconf->fallback_to_proxy_http = 2 wouldn't work as
expected, maybe arg is always 0 or 1 but some compilers warn with
-Woverflow here anyway.
>
> > + dconf->fallback_to_proxy_http_set = 1;
> > + return NULL;
> > +}
> > +
> > 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 =
> > + can_fallback_to_proxy_http =
> > (ap_find_linked_module("mod_proxy_http.c") != NULL);
> >
> > return OK;
>
> > @@ -467,6 +507,7 @@ static const command_rec ws_proxy_cmds[]
> > AP_INIT_TAKE1("ProxyWebsocketAsyncDelay", proxyws_set_asynch_delay, NULL,
> > RSRC_CONF|ACCESS_CONF,
> > "amount of time to poll before going asynchronous"),
> > +
>
> Stray empty line?
Well there was one between the existing directives, so I thought I'd
separate the new and {NULL} ones with an empty line too.
>
> > {NULL}
> > };
Regards;
Yann.