On 8/30/21 8:04 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Mon Aug 30 18:04:20 2021
> New Revision: 1892740
>
> URL: http://svn.apache.org/viewvc?rev=1892740&view=rev
> Log:
> mod_proxy: Fix potential tunneling infinite loop and spurious timeout.
> PRs 65521 and 65519.
>
> * modules/proxy/proxy_util.c(ap_proxy_tunnel_run):
> Avoid an infinite loop by shutting down the connection for write when poll()
> returns POLLHUP and read is already down. PR 65521.
>
> * modules/proxy/proxy_util.c(ap_proxy_tunnel_run):
> When write completion is finished don't check for ap_filter_input_pending()
> before proxy_tunnel_forward() to flush input data, this is a nonblocking
> read
> already which will do the same thing implicitely. ap_filter_input_pending()
> is broken in 2.4.x without the whole pending data mechanism (not backported
> yet), so let's align here. PR 65519.
>
>
> Added:
> httpd/httpd/trunk/changes-entries/ap_proxy_tunnel_run.txt
> 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=1892740&r1=1892739&r2=1892740&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Aug 30 18:04:20 2021
> @@ -4890,12 +4890,16 @@ PROXY_DECLARE(int) ap_proxy_tunnel_run(p
> return HTTP_INTERNAL_SERVER_ERROR;
> }
>
> - /* Write if we asked for POLLOUT, and got it or POLLERR
> - * alone (i.e. not with POLLIN|HUP). We want the output filters
> - * to know about the socket error if any, by failing the write.
> + /* We want to write if we asked for POLLOUT and got:
> + * - POLLOUT: the socket is ready for write;
> + * - !POLLIN: the socket is in error state (POLLERR) so we let
> + * the user know by failing the write and log, OR the socket
> + * is shutdown for read already (POLLHUP) so we have to
> + * shutdown for write.
> */
> if ((tc->pfd->reqevents & APR_POLLOUT)
> && ((pfd->rtnevents & APR_POLLOUT)
> + || !(tc->pfd->reqevents & APR_POLLIN)
Why should we write if we requested POLLOUT did not get POLLOUT back, but did
not request for POLLIN?
> || !(pfd->rtnevents & (APR_POLLIN | APR_POLLHUP)))) {
> struct proxy_tunnel_conn *out = tc, *in = tc->other;
>
> @@ -4944,12 +4948,25 @@ PROXY_DECLARE(int) ap_proxy_tunnel_run(p
> return rc;
> }
> }
> +
> + /* Flush any pending input data now, we don't know when
> + * the next POLLIN will trigger and retaining data might
> + * deadlock the underlying protocol. We don't check for
> + * pending data first with ap_filter_input_pending()
> since
> + * the read from proxy_tunnel_forward() is nonblocking
> + * anyway and returning OK if there's no data.
> + */
> + rc = proxy_tunnel_forward(tunnel, in);
> + if (rc != OK) {
> + return rc;
> + }
Don't we do all of this already a few lines above if
ap_filter_input_pending(in->c) == OK?
Why doing it again?
Regards
Rüdiger
