On 11/03/2019 04:48 PM, yla...@apache.org wrote:
> Author: ylavic
> Date: Sun Nov 3 15:48:53 2019
> New Revision: 1869338
>
> URL: http://svn.apache.org/viewvc?rev=1869338&view=rev
> Log:
> mod_proxy: factorize mod_proxy_{connect,wstunnel} tunneling code in
> proxy_util.
>
> This commit adds struct proxy_tunnel_rec that contains the fields needed for a
> poll() loop through the filters chains, plus functions
> ap_proxy_tunnel_create()
> and ap_proxy_tunnel_run() to respectively initialize a tunnel and (re)start
> it.
>
> Proxy connect and wstunnel modules now make use of this new API to avoid
> duplicating logic and code.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/log-message-tags/next-number
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/modules/proxy/mod_proxy.h
> httpd/httpd/trunk/modules/proxy/mod_proxy_connect.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c
> httpd/httpd/trunk/modules/proxy/proxy_util.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=1869338&r1=1869337&r2=1869338&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_wstunnel.c Sun Nov 3 15:48:53
> 2019
> @@ -480,17 +299,26 @@ static int proxy_wstunnel_handler(reques
> return DECLINED;
> }
>
> + /* XXX: what's the point of "NONE"? We probably should _always_ check
> + * that the client wants an Upgrade..
> + */
NONE was used to upgrade the client whether it requested it or no idea what the
purpose was.
Maybe Jean-Frederic who introduced it in r1792092 can explain.
> + upgrade_method = *worker->s->upgrade ? worker->s->upgrade : "WebSocket";
> if (ap_cstr_casecmp(upgrade_method, "NONE") != 0) {
> - const char *upgrade;
> upgrade = apr_table_get(r->headers_in, "Upgrade");
> if (!upgrade || (ap_cstr_casecmp(upgrade, upgrade_method) != 0 &&
> - ap_cstr_casecmp(upgrade_method, "ANY") !=0)) {
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02900)
> - "declining URL %s (not %s, Upgrade: header is
> %s)",
> - url, upgrade_method, upgrade ? upgrade :
> "missing");
> - return DECLINED;
> + ap_cstr_casecmp(upgrade_method, "ANY") != 0)) {
> + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02900)
> + "require upgrade for URL %s "
> + "(Upgrade header is %s, expecting %s)",
> + url, upgrade ? upgrade : "missing",
> upgrade_method);
> + apr_table_setn(r->err_headers_out, "Connection", "Upgrade");
> + apr_table_setn(r->err_headers_out, "Upgrade", upgrade_method);
> + return HTTP_UPGRADE_REQUIRED;
Why don't we fallback any longer to normal HTTP handling if no upgrade handler
was sent?
This was itruduced by you in r1674632
> }
> }
> + else {
> + upgrade = "WebSocket";
> + }
>
> uri = apr_palloc(p, sizeof(*uri));
> ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02451) "serving URL
> %s", url);
Regards
Rüdiger
