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 (file)
index 0000000..c31cfcd
--- /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 31fdf43d77faa811a99099b38838f40de713cc48..3dd57d3f77e2d6162f245f42a258bd72823f74b7 100644 (file)
--- 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 812c32f35842e9ee8c6f4601d19d56599ca38b41..b65859317e7287be1a15a54f5d5bcef3169e40e9 100644 (file)
--- 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))