Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On Mon, Jul 3, 2023 at 11:10 AM Ruediger Pluem wrote: > > > > On 7/3/23 10:17 AM, Stefan Eissing via dev wrote: > > > > > >> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev > >> : > >> > >> > >> > >>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem : > >>> > >>> > >>> > >>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote: > > > > Am 30.06.2023 um 15:56 schrieb Yann Ylavic : > > > > On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem > > wrote: > >> > >> On 6/30/23 11:08 AM, ic...@apache.org wrote: > >>> Author: icing > >>> Date: Fri Jun 30 09:08:23 2023 > >>> New Revision: 1910704 > >>> > >>> URL: http://svn.apache.org/viewvc?rev=1910704=rev > >>> Log: > >>> proxy: in proxy tunnels, use the smaller timeout value of > >>> client and origin as timeout for polling the tunnel. > >>> > >>> > >>> Modified: > >>> httpd/httpd/trunk/modules/proxy/proxy_util.c > >>> > >>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c > >>> URL: > >>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff > >>> == > >>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) > >>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 > >>> 2023 > >>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun > >>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, > >>> _timeout); > >>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); > >>> > >>> -/* Defaults to the biggest timeout of both connections */ > >>> -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > > >>> client_timeout)? > >>> - origin_timeout : client_timeout; > >>> +/* Defaults to the smallest timeout of both connections */ > >>> +tunnel->timeout = (client_timeout >= 0 && client_timeout < > >>> origin_timeout ? > >>> + client_timeout : origin_timeout); > >> > >> Why? > > > > We discussed this (quickly) with Stefan on > > https://github.com/apache/httpd/pull/366, but hey the commit is here > > for review finally :) > > > >> It was the other way round on purpose, e.g. if Timeout is set to 5 for > >> a small front end timeout and ProxyTimeout is set to > >> e.g. 600 to keep Websockets open for 10 minutes. > > > > It seems to me that using Timeout (5s) here is a valid case too if > > Timeout < ProxyTimeout (as in your example) is a way to limit how long > > a client can consume httpd resources. > > So maybe we should only use the backend timeout which is an easy(er) > > way for the user to control this? > > So, the goal is to allow someone keeping the websocket open for longer > than we usually allow for HTTP requests, to set a long ProxyTimeout or > timeout parameter to ProxyPass. > > For HTTP/1.1 that would override the connection Timeout, since the tunnel > poll would only use the largest value. > > For HTTP/2 I have to check how that how to accomplish that. The working > there is different. > >>> > >>> Sorry for being a bit grumpy above, but this came out of the blue for me. > >>> It breaks an important use case for me and the use case > >>> for the other way round was not clear to me. Maybe we can find a solution > >>> that addresses both use cases at best by default and > >>> automatically. Any pointers for your use case such that I can have a look > >>> and be more constructive :-). > >>> > >> > >> Thanks that you spoke up! I heard no grumpiness. ;) > > > > Ok, testing how things *REALLY* work here, I stumbled upon: > > > > mod_proxy_http.c:1570 > > /* Let proxy tunnel forward everything within this thread */ > > req->tunnel->timeout = req->idle_timeout; > > status = ap_proxy_tunnel_run(req->tunnel); > > > > which overrides everything we discussed. So, how do we want this to work > > and who has the final say? > > Very good catch. If you look at > https://github.com/apache/httpd/blob/cc0735d1cde406c8d15016e4b2afcebb4d11fc87/modules/proxy/mod_proxy_http.c#L2009-L2023 > > then in the proxy case the backend timeout seems to win. Hence my crying was > wrong and in the http proxy case things still work as > before. The above was for trunk. In case of 2.4.x we also override the > setting from ap_proxy_tunnel_create here: > https://github.com/apache/httpd/blob/aefc30086aa71f24f77290deb88f949f36575c05/modules/proxy/mod_proxy_http.c#L1538-L1549 > in the > same fashion as we set it in ap_proxy_tunnel_create: The longest one wins. > > What is actually your use case to use the shortest one? The new WebSocket > over HTTP/2 feature? Do we get hit by the fact that on > HTTP/2 we have multiple conn_recs / streams over one
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
> Am 03.07.2023 um 11:09 schrieb Ruediger Pluem : > > > > On 7/3/23 10:17 AM, Stefan Eissing via dev wrote: >> >> >>> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev >>> : >>> >>> >>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem : On 6/30/23 4:15 PM, Stefan Eissing via dev wrote: > > >> Am 30.06.2023 um 15:56 schrieb Yann Ylavic : >> >> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: >>> >>> On 6/30/23 11:08 AM, ic...@apache.org wrote: Author: icing Date: Fri Jun 30 09:08:23 2023 New Revision: 1910704 URL: http://svn.apache.org/viewvc?rev=1910704=rev Log: proxy: in proxy tunnels, use the smaller timeout value of client and origin as timeout for polling the tunnel. Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff == --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); -/* Defaults to the biggest timeout of both connections */ -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)? - origin_timeout : client_timeout; +/* Defaults to the smallest timeout of both connections */ +tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ? + client_timeout : origin_timeout); >>> >>> Why? >> >> We discussed this (quickly) with Stefan on >> https://github.com/apache/httpd/pull/366, but hey the commit is here >> for review finally :) >> >>> It was the other way round on purpose, e.g. if Timeout is set to 5 for >>> a small front end timeout and ProxyTimeout is set to >>> e.g. 600 to keep Websockets open for 10 minutes. >> >> It seems to me that using Timeout (5s) here is a valid case too if >> Timeout < ProxyTimeout (as in your example) is a way to limit how long >> a client can consume httpd resources. >> So maybe we should only use the backend timeout which is an easy(er) >> way for the user to control this? > > So, the goal is to allow someone keeping the websocket open for longer > than we usually allow for HTTP requests, to set a long ProxyTimeout or > timeout parameter to ProxyPass. > > For HTTP/1.1 that would override the connection Timeout, since the tunnel > poll would only use the largest value. > > For HTTP/2 I have to check how that how to accomplish that. The working > there is different. Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and automatically. Any pointers for your use case such that I can have a look and be more constructive :-). >>> >>> Thanks that you spoke up! I heard no grumpiness. ;) >> >> Ok, testing how things *REALLY* work here, I stumbled upon: >> >> mod_proxy_http.c:1570 >>/* Let proxy tunnel forward everything within this thread */ >>req->tunnel->timeout = req->idle_timeout; >>status = ap_proxy_tunnel_run(req->tunnel); >> >> which overrides everything we discussed. So, how do we want this to work and >> who has the final say? > > Very good catch. If you look at > https://github.com/apache/httpd/blob/cc0735d1cde406c8d15016e4b2afcebb4d11fc87/modules/proxy/mod_proxy_http.c#L2009-L2023 > > then in the proxy case the backend timeout seems to win. Hence my crying was > wrong and in the http proxy case things still work as > before. The above was for trunk. In case of 2.4.x we also override the > setting from ap_proxy_tunnel_create here: > https://github.com/apache/httpd/blob/aefc30086aa71f24f77290deb88f949f36575c05/modules/proxy/mod_proxy_http.c#L1538-L1549 > in the > same fashion as we set it in ap_proxy_tunnel_create: The longest one wins. > > What is actually your use case to use the shortest one? The new WebSocket > over HTTP/2 feature? Do we get hit by the fact that on > HTTP/2 we have multiple conn_recs / streams over one TCP connection? No actual use case. I am just looking to make it work with h2 exactly as
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On 7/3/23 10:17 AM, Stefan Eissing via dev wrote: > > >> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev : >> >> >> >>> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem : >>> >>> >>> >>> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote: > Am 30.06.2023 um 15:56 schrieb Yann Ylavic : > > On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: >> >> On 6/30/23 11:08 AM, ic...@apache.org wrote: >>> Author: icing >>> Date: Fri Jun 30 09:08:23 2023 >>> New Revision: 1910704 >>> >>> URL: http://svn.apache.org/viewvc?rev=1910704=rev >>> Log: >>> proxy: in proxy tunnels, use the smaller timeout value of >>> client and origin as timeout for polling the tunnel. >>> >>> >>> Modified: >>> httpd/httpd/trunk/modules/proxy/proxy_util.c >>> >>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c >>> URL: >>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff >>> == >>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) >>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 >>> 2023 >>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun >>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); >>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); >>> >>> -/* Defaults to the biggest timeout of both connections */ >>> -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > >>> client_timeout)? >>> - origin_timeout : client_timeout; >>> +/* Defaults to the smallest timeout of both connections */ >>> +tunnel->timeout = (client_timeout >= 0 && client_timeout < >>> origin_timeout ? >>> + client_timeout : origin_timeout); >> >> Why? > > We discussed this (quickly) with Stefan on > https://github.com/apache/httpd/pull/366, but hey the commit is here > for review finally :) > >> It was the other way round on purpose, e.g. if Timeout is set to 5 for a >> small front end timeout and ProxyTimeout is set to >> e.g. 600 to keep Websockets open for 10 minutes. > > It seems to me that using Timeout (5s) here is a valid case too if > Timeout < ProxyTimeout (as in your example) is a way to limit how long > a client can consume httpd resources. > So maybe we should only use the backend timeout which is an easy(er) > way for the user to control this? So, the goal is to allow someone keeping the websocket open for longer than we usually allow for HTTP requests, to set a long ProxyTimeout or timeout parameter to ProxyPass. For HTTP/1.1 that would override the connection Timeout, since the tunnel poll would only use the largest value. For HTTP/2 I have to check how that how to accomplish that. The working there is different. >>> >>> Sorry for being a bit grumpy above, but this came out of the blue for me. >>> It breaks an important use case for me and the use case >>> for the other way round was not clear to me. Maybe we can find a solution >>> that addresses both use cases at best by default and >>> automatically. Any pointers for your use case such that I can have a look >>> and be more constructive :-). >>> >> >> Thanks that you spoke up! I heard no grumpiness. ;) > > Ok, testing how things *REALLY* work here, I stumbled upon: > > mod_proxy_http.c:1570 > /* Let proxy tunnel forward everything within this thread */ > req->tunnel->timeout = req->idle_timeout; > status = ap_proxy_tunnel_run(req->tunnel); > > which overrides everything we discussed. So, how do we want this to work and > who has the final say? Very good catch. If you look at https://github.com/apache/httpd/blob/cc0735d1cde406c8d15016e4b2afcebb4d11fc87/modules/proxy/mod_proxy_http.c#L2009-L2023 then in the proxy case the backend timeout seems to win. Hence my crying was wrong and in the http proxy case things still work as before. The above was for trunk. In case of 2.4.x we also override the setting from ap_proxy_tunnel_create here: https://github.com/apache/httpd/blob/aefc30086aa71f24f77290deb88f949f36575c05/modules/proxy/mod_proxy_http.c#L1538-L1549 in the same fashion as we set it in ap_proxy_tunnel_create: The longest one wins. What is actually your use case to use the shortest one? The new WebSocket over HTTP/2 feature? Do we get hit by the fact that on HTTP/2 we have multiple conn_recs / streams over one TCP connection? Regards Rüdiger
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
> Am 30.06.2023 um 17:22 schrieb Stefan Eissing via dev : > > > >> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem : >> >> >> >> On 6/30/23 4:15 PM, Stefan Eissing via dev wrote: >>> >>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic : On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: > > On 6/30/23 11:08 AM, ic...@apache.org wrote: >> Author: icing >> Date: Fri Jun 30 09:08:23 2023 >> New Revision: 1910704 >> >> URL: http://svn.apache.org/viewvc?rev=1910704=rev >> Log: >> proxy: in proxy tunnels, use the smaller timeout value of >> client and origin as timeout for polling the tunnel. >> >> >> Modified: >> httpd/httpd/trunk/modules/proxy/proxy_util.c >> >> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c >> URL: >> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff >> == >> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) >> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 >> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun >> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); >> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); >> >> -/* Defaults to the biggest timeout of both connections */ >> -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > >> client_timeout)? >> - origin_timeout : client_timeout; >> +/* Defaults to the smallest timeout of both connections */ >> +tunnel->timeout = (client_timeout >= 0 && client_timeout < >> origin_timeout ? >> + client_timeout : origin_timeout); > > Why? We discussed this (quickly) with Stefan on https://github.com/apache/httpd/pull/366, but hey the commit is here for review finally :) > It was the other way round on purpose, e.g. if Timeout is set to 5 for a > small front end timeout and ProxyTimeout is set to > e.g. 600 to keep Websockets open for 10 minutes. It seems to me that using Timeout (5s) here is a valid case too if Timeout < ProxyTimeout (as in your example) is a way to limit how long a client can consume httpd resources. So maybe we should only use the backend timeout which is an easy(er) way for the user to control this? >>> >>> So, the goal is to allow someone keeping the websocket open for longer >>> than we usually allow for HTTP requests, to set a long ProxyTimeout or >>> timeout parameter to ProxyPass. >>> >>> For HTTP/1.1 that would override the connection Timeout, since the tunnel >>> poll would only use the largest value. >>> >>> For HTTP/2 I have to check how that how to accomplish that. The working >>> there is different. >> >> Sorry for being a bit grumpy above, but this came out of the blue for me. It >> breaks an important use case for me and the use case >> for the other way round was not clear to me. Maybe we can find a solution >> that addresses both use cases at best by default and >> automatically. Any pointers for your use case such that I can have a look >> and be more constructive :-). >> > > Thanks that you spoke up! I heard no grumpiness. ;) Ok, testing how things *REALLY* work here, I stumbled upon: mod_proxy_http.c:1570 /* Let proxy tunnel forward everything within this thread */ req->tunnel->timeout = req->idle_timeout; status = ap_proxy_tunnel_run(req->tunnel); which overrides everything we discussed. So, how do we want this to work and who has the final say? - Stefan
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
> Am 30.06.2023 um 16:45 schrieb Ruediger Pluem : > > > > On 6/30/23 4:15 PM, Stefan Eissing via dev wrote: >> >> >>> Am 30.06.2023 um 15:56 schrieb Yann Ylavic : >>> >>> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: On 6/30/23 11:08 AM, ic...@apache.org wrote: > Author: icing > Date: Fri Jun 30 09:08:23 2023 > New Revision: 1910704 > > URL: http://svn.apache.org/viewvc?rev=1910704=rev > Log: > proxy: in proxy tunnels, use the smaller timeout value of > client and origin as timeout for polling the tunnel. > > > Modified: > httpd/httpd/trunk/modules/proxy/proxy_util.c > > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff > == > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 > @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun >apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); >apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); > > -/* Defaults to the biggest timeout of both connections */ > -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > > client_timeout)? > - origin_timeout : client_timeout; > +/* Defaults to the smallest timeout of both connections */ > +tunnel->timeout = (client_timeout >= 0 && client_timeout < > origin_timeout ? > + client_timeout : origin_timeout); Why? >>> >>> We discussed this (quickly) with Stefan on >>> https://github.com/apache/httpd/pull/366, but hey the commit is here >>> for review finally :) >>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to e.g. 600 to keep Websockets open for 10 minutes. >>> >>> It seems to me that using Timeout (5s) here is a valid case too if >>> Timeout < ProxyTimeout (as in your example) is a way to limit how long >>> a client can consume httpd resources. >>> So maybe we should only use the backend timeout which is an easy(er) >>> way for the user to control this? >> >> So, the goal is to allow someone keeping the websocket open for longer >> than we usually allow for HTTP requests, to set a long ProxyTimeout or >> timeout parameter to ProxyPass. >> >> For HTTP/1.1 that would override the connection Timeout, since the tunnel >> poll would only use the largest value. >> >> For HTTP/2 I have to check how that how to accomplish that. The working >> there is different. > > Sorry for being a bit grumpy above, but this came out of the blue for me. It > breaks an important use case for me and the use case > for the other way round was not clear to me. Maybe we can find a solution > that addresses both use cases at best by default and > automatically. Any pointers for your use case such that I can have a look and > be more constructive :-). > Thanks that you spoke up! I heard no grumpiness. ;)
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On 6/30/23 4:15 PM, Stefan Eissing via dev wrote: > > >> Am 30.06.2023 um 15:56 schrieb Yann Ylavic : >> >> On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: >>> >>> On 6/30/23 11:08 AM, ic...@apache.org wrote: Author: icing Date: Fri Jun 30 09:08:23 2023 New Revision: 1910704 URL: http://svn.apache.org/viewvc?rev=1910704=rev Log: proxy: in proxy tunnels, use the smaller timeout value of client and origin as timeout for polling the tunnel. Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff == --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); -/* Defaults to the biggest timeout of both connections */ -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > client_timeout)? - origin_timeout : client_timeout; +/* Defaults to the smallest timeout of both connections */ +tunnel->timeout = (client_timeout >= 0 && client_timeout < origin_timeout ? + client_timeout : origin_timeout); >>> >>> Why? >> >> We discussed this (quickly) with Stefan on >> https://github.com/apache/httpd/pull/366, but hey the commit is here >> for review finally :) >> >>> It was the other way round on purpose, e.g. if Timeout is set to 5 for a >>> small front end timeout and ProxyTimeout is set to >>> e.g. 600 to keep Websockets open for 10 minutes. >> >> It seems to me that using Timeout (5s) here is a valid case too if >> Timeout < ProxyTimeout (as in your example) is a way to limit how long >> a client can consume httpd resources. >> So maybe we should only use the backend timeout which is an easy(er) >> way for the user to control this? > > So, the goal is to allow someone keeping the websocket open for longer > than we usually allow for HTTP requests, to set a long ProxyTimeout or > timeout parameter to ProxyPass. > > For HTTP/1.1 that would override the connection Timeout, since the tunnel > poll would only use the largest value. > > For HTTP/2 I have to check how that how to accomplish that. The working > there is different. Sorry for being a bit grumpy above, but this came out of the blue for me. It breaks an important use case for me and the use case for the other way round was not clear to me. Maybe we can find a solution that addresses both use cases at best by default and automatically. Any pointers for your use case such that I can have a look and be more constructive :-). Regards Rüdiger
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
> Am 30.06.2023 um 15:56 schrieb Yann Ylavic : > > On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: >> >> On 6/30/23 11:08 AM, ic...@apache.org wrote: >>> Author: icing >>> Date: Fri Jun 30 09:08:23 2023 >>> New Revision: 1910704 >>> >>> URL: http://svn.apache.org/viewvc?rev=1910704=rev >>> Log: >>> proxy: in proxy tunnels, use the smaller timeout value of >>> client and origin as timeout for polling the tunnel. >>> >>> >>> Modified: >>>httpd/httpd/trunk/modules/proxy/proxy_util.c >>> >>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c >>> URL: >>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff >>> == >>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) >>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 >>> @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun >>> apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); >>> apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); >>> >>> -/* Defaults to the biggest timeout of both connections */ >>> -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > >>> client_timeout)? >>> - origin_timeout : client_timeout; >>> +/* Defaults to the smallest timeout of both connections */ >>> +tunnel->timeout = (client_timeout >= 0 && client_timeout < >>> origin_timeout ? >>> + client_timeout : origin_timeout); >> >> Why? > > We discussed this (quickly) with Stefan on > https://github.com/apache/httpd/pull/366, but hey the commit is here > for review finally :) > >> It was the other way round on purpose, e.g. if Timeout is set to 5 for a >> small front end timeout and ProxyTimeout is set to >> e.g. 600 to keep Websockets open for 10 minutes. > > It seems to me that using Timeout (5s) here is a valid case too if > Timeout < ProxyTimeout (as in your example) is a way to limit how long > a client can consume httpd resources. > So maybe we should only use the backend timeout which is an easy(er) > way for the user to control this? So, the goal is to allow someone keeping the websocket open for longer than we usually allow for HTTP requests, to set a long ProxyTimeout or timeout parameter to ProxyPass. For HTTP/1.1 that would override the connection Timeout, since the tunnel poll would only use the largest value. For HTTP/2 I have to check how that how to accomplish that. The working there is different.
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On Fri, Jun 30, 2023 at 2:44 PM Ruediger Pluem wrote: > > On 6/30/23 11:08 AM, ic...@apache.org wrote: > > Author: icing > > Date: Fri Jun 30 09:08:23 2023 > > New Revision: 1910704 > > > > URL: http://svn.apache.org/viewvc?rev=1910704=rev > > Log: > > proxy: in proxy tunnels, use the smaller timeout value of > >client and origin as timeout for polling the tunnel. > > > > > > Modified: > > httpd/httpd/trunk/modules/proxy/proxy_util.c > > > > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c > > URL: > > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff > > == > > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) > > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 > > @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun > > apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); > > apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); > > > > -/* Defaults to the biggest timeout of both connections */ > > -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > > > client_timeout)? > > - origin_timeout : client_timeout; > > +/* Defaults to the smallest timeout of both connections */ > > +tunnel->timeout = (client_timeout >= 0 && client_timeout < > > origin_timeout ? > > + client_timeout : origin_timeout); > > Why? We discussed this (quickly) with Stefan on https://github.com/apache/httpd/pull/366, but hey the commit is here for review finally :) > It was the other way round on purpose, e.g. if Timeout is set to 5 for a > small front end timeout and ProxyTimeout is set to > e.g. 600 to keep Websockets open for 10 minutes. It seems to me that using Timeout (5s) here is a valid case too if Timeout < ProxyTimeout (as in your example) is a way to limit how long a client can consume httpd resources. So maybe we should only use the backend timeout which is an easy(er) way for the user to control this? Regards; Yann.
Re: svn commit: r1910704 - /httpd/httpd/trunk/modules/proxy/proxy_util.c
On 6/30/23 11:08 AM, ic...@apache.org wrote: > Author: icing > Date: Fri Jun 30 09:08:23 2023 > New Revision: 1910704 > > URL: http://svn.apache.org/viewvc?rev=1910704=rev > Log: > proxy: in proxy tunnels, use the smaller timeout value of >client and origin as timeout for polling the tunnel. > > > Modified: > httpd/httpd/trunk/modules/proxy/proxy_util.c > > Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1910704=1910703=1910704=diff > == > --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original) > +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Fri Jun 30 09:08:23 2023 > @@ -4921,9 +4921,9 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tun > apr_socket_timeout_get(tunnel->origin->pfd->desc.s, _timeout); > apr_socket_opt_set(tunnel->origin->pfd->desc.s, APR_SO_NONBLOCK, 1); > > -/* Defaults to the biggest timeout of both connections */ > -tunnel->timeout = (origin_timeout >= 0 && origin_timeout > > client_timeout)? > - origin_timeout : client_timeout; > +/* Defaults to the smallest timeout of both connections */ > +tunnel->timeout = (client_timeout >= 0 && client_timeout < > origin_timeout ? > + client_timeout : origin_timeout); Why? It was the other way round on purpose, e.g. if Timeout is set to 5 for a small front end timeout and ProxyTimeout is set to e.g. 600 to keep Websockets open for 10 minutes. Regards Rüdiger
