From: Yann Ylavic Date: Mon, 6 Sep 2021 08:34:19 +0000 (+0000) Subject: Merge r1892740, r1892851 from trunk: X-Git-Tag: candidate-2.4.49-rc1~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4ef2a7fcff9258bce2eb21a1f5b9f7481286308c;p=thirdparty%2Fapache%2Fhttpd.git Merge r1892740, r1892851 from trunk: 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. mod_proxy: Follow up to r1892740. Really remove the old ap_filter_input_pending() handling forgotten by r1892740. Submitted by: ylavic Reviewed by: ylavic, covener, jorton git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1892971 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/changes-entries/ap_proxy_tunnel_run.txt b/changes-entries/ap_proxy_tunnel_run.txt new file mode 100644 index 00000000000..c31cfcd3743 --- /dev/null +++ b/changes-entries/ap_proxy_tunnel_run.txt @@ -0,0 +1,4 @@ + *) mod_proxy: Fix a potential infinite loop when tunneling Upgrade(d) + protocols from mod_proxy_http, and a timeout triggering falsely when + using mod_proxy_wstunnel, mod_proxy_connect or mod_proxy_http with + upgrade= setting. PRs 65521 and 65519. [Yann Ylavic] \ No newline at end of file diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c index 31fdf43d77f..3dd57d3f77e 100644 --- a/modules/proxy/mod_proxy_http.c +++ b/modules/proxy/mod_proxy_http.c @@ -1479,7 +1479,7 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(10239) "HTTP: tunneling protocol %s", upgrade); - rv = ap_proxy_tunnel_create(&tunnel, r, origin, "HTTP"); + rv = ap_proxy_tunnel_create(&tunnel, r, origin, upgrade); if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(10240) "can't create tunnel for %s", upgrade); diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c index 812c32f3584..b65859317e7 100644 --- a/modules/proxy/proxy_util.c +++ b/modules/proxy/proxy_util.c @@ -4918,12 +4918,16 @@ PROXY_DECLARE(int) ap_proxy_tunnel_run(proxy_tunnel_rec *tunnel) 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) || !(pfd->rtnevents & (APR_POLLIN | APR_POLLHUP)))) { struct proxy_tunnel_conn *out = tc, *in = tc->other; @@ -4964,20 +4968,22 @@ PROXY_DECLARE(int) ap_proxy_tunnel_run(proxy_tunnel_rec *tunnel) /* Flush any pending input data now, we don't know when * the next POLLIN will trigger and retaining data might - * block the protocol. + * 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. */ - if (ap_filter_input_pending(in->c) == OK) { - rc = proxy_tunnel_forward(tunnel, in); - if (rc != OK) { - return rc; - } + rc = proxy_tunnel_forward(tunnel, in); + if (rc != OK) { + return rc; } } } - /* Read if we asked for POLLIN|HUP, and got it or POLLERR - * alone (i.e. not with POLLOUT). We want the input filters - * to know about the socket error if any, by failing the read. + /* We want to read if we asked for POLLIN|HUP and got: + * - POLLIN|HUP: the socket is ready for read or EOF (POLLHUP); + * - !POLLOUT: the socket is in error state (POLLERR) so we let + * the user know by failing the read and log. */ if ((tc->pfd->reqevents & APR_POLLIN) && ((pfd->rtnevents & (APR_POLLIN | APR_POLLHUP))