On 5/12/20 2:20 PM, [email protected] wrote:
> Author: ylavic
> Date: Tue May 12 12:20:57 2020
> New Revision: 1877646
>
> URL: http://svn.apache.org/viewvc?rev=1877646&view=rev
> Log:
> mod_proxy_http: handle Upgrade requests and upgraded protocol forwarding.
>
> If the request Upgrade header matches the worker upgrade= parameter and
> the backend switches the protocol, do the tunneling in mod_proxy_http.
> This allows to keep the protocol to HTTP until the backend really
> switches the protocol, and apply usual output filters.
>
> When configured to forward Upgrade mechanism, we want the backend to be
> able to announce its Upgrade protocol to the client (e.g. with 426
> Upgrade Required response) and thus forward back the Upgrade header that
> matches the one(s) configured in the worker upgrade= parameter.
>
> modules/proxy/mod_proxy.h:
> modules/proxy/proxy_util.c:
> ap_proxy_worker_can_upgrade(): added helper to determine whether a
> proxy worker is configured to forward an Upgrade protocol.
>
> include/ap_mmn.h:
> Bump MMN minor for ap_proxy_worker_can_upgrade().
>
> modules/proxy/mod_proxy.c:
> set_worker_param(): handle worker parameter upgrade=ANY as upgrade=*
> (should the "any" protocol scheme be something some day..).
>
> modules/proxy/mod_proxy_wstunnel.c:
> proxy_wstunnel_handler(): use ap_proxy_worker_can_upgrade() to match
> the Upgrade header. Axe handling of upgrade=NONE, it makes no sense to
> Upgrade a connection if the client did not ask for it, nor to configure
> mod_proxy_wstunnel to use a worker with upgrade=NONE by the way.
>
> modules/proxy/mod_proxy_http.c:
> proxy_http_req_t: add fields force10 (force HTTP/1.0) and upgrade (value
> of the Upgrade header sent by the client if it matches the configuration,
> NULL otherwise).
> proxy_http_handler(): use ap_proxy_worker_can_upgrade() to determine
> whether the request is electable for end to end protocol upgrading and set
> req->upgrade accordingly.
> terminate_headers(): handle Connection and Upgrade headers to send to the
> backend, according to req->force10 and req->upgrade set before.
> ap_proxy_http_prefetch(): use req->force10 and terminate_headers().
> send_continue_body(): added helper to send the body retained for end to
> end 100-continue handling.
> ap_proxy_http_process_response(): use ap_proxy_worker_can_upgrade() to
> match the response Upgrade header and forward it back if it matches the
> configured one(s). That is for 101 Switching Protocol obviously but also
> any other status code which is not overidden, at the backend wish. If the
> protocol is switching, create a proxy tunnel and run it, using the minimal
> timeout from the client or backend connection.
>
> Github: closes #125
>
>
> 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.c
> httpd/httpd/trunk/modules/proxy/mod_proxy.h
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.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_http.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1877646&r1=1877645&r2=1877646&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Tue May 12 12:20:57 2020
> @@ -1509,6 +1564,7 @@ int ap_proxy_http_process_response(proxy
> ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
> "HTTP: received interim %d response", r->status);
> if (!policy
> + || upgrade
Shouldn't that be (upgrade && proxy_status == HTTP_SWITCHING_PROTOCOLS) in case
someone sends a upgrade header
without status HTTP_SWITCHING_PROTOCOLS?
Furthermore don't we need to add in back the upgrade header and the Connection:
upgrade before we send the interim
response with HTTP_SWITCHING_PROTOCOLS?
> || (!strcasecmp(policy, "RFC")
> && (proxy_status != HTTP_CONTINUE
> || (req->expecting_100 = 1)))) {
> @@ -1615,6 +1649,67 @@ int ap_proxy_http_process_response(proxy
> do_100_continue = 0;
> }
>
> + if (proxy_status == HTTP_SWITCHING_PROTOCOLS) {
> + apr_status_t rv;
> + proxy_tunnel_rec *tunnel;
> + apr_interval_time_t client_timeout = -1,
> + backend_timeout = -1;
> +
> + /* If we didn't send the full body yet, do it now */
> + if (do_100_continue) {
> + req->expecting_100 = 0;
> + status = send_continue_body(req);
Can we really have a body that we held back in case the backend sends
HTTP_SWITCHING_PROTOCOLS?
IMHO if we have a body held back we sent an Expect 100-Continue header with our
request to the backend
and the backend should have replied with 100-Continue and not with
HTTP_SWITCHING_PROTOCOLS
> + if (status != OK) {
> + return status;
> + }
> + }
> +
> + 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");
> + if (rv != APR_SUCCESS) {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(10240)
> + "can't create tunnel for %s", upgrade);
> + return HTTP_INTERNAL_SERVER_ERROR;
> + }
> +
> + /* Set timeout to the lowest configured for client or backend */
> + apr_socket_timeout_get(backend->sock, &backend_timeout);
> + apr_socket_timeout_get(ap_get_conn_socket(c), &client_timeout);
> + if (backend_timeout >= 0 && backend_timeout < client_timeout) {
> + tunnel->timeout = backend_timeout;
> + }
> + else {
> + tunnel->timeout = client_timeout;
> + }
> +
> + /* Bidirectional non-HTTP stream will confuse mod_reqtimeoout, we
> + * use a single idle timeout from now on.
> + */
> + ap_remove_input_filter_byhandle(c->input_filters, "reqtimeout");
> +
> + /* Let proxy tunnel forward everything */
> + status = ap_proxy_tunnel_run(tunnel);
> + if (ap_is_HTTP_ERROR(status)) {
> + /* Tunnel always return HTTP_GATEWAY_TIME_OUT on timeout,
> + * but we can differentiate between client and backend here.
> + */
> + if (status == HTTP_GATEWAY_TIME_OUT
> + && tunnel->timeout == client_timeout) {
> + status = HTTP_REQUEST_TIME_OUT;
> + }
> + }
> + else {
> + /* Update r->status for custom log */
> + status = HTTP_SWITCHING_PROTOCOLS;
> + }
> + r->status = status;
> +
> + /* We are done with both connections */
> + return DONE;
> + }
> +
> if (interim_response) {
> /* Already forwarded above, read next response */
> continue;
> @@ -1666,6 +1761,12 @@ int ap_proxy_http_process_response(proxy
> return proxy_status;
> }
>
> + /* Forward back Upgrade header if it matches the configured one(s).
> */
> + if (upgrade && ap_proxy_worker_can_upgrade(p, worker, upgrade)) {
> + apr_table_setn(r->headers_out, "Connection", "Upgrade");
> + apr_table_setn(r->headers_out, "Upgrade", apr_pstrdup(p,
> upgrade));
> + }
> +
Hmm. When do we hit this? When the backend sends HTTP_UPGRADE_REQUIRED? And if
yes shouldn't we only care in this case?
> r->sent_bodyct = 1;
> /*
> * Is it an HTTP/0.9 response or did we maybe preread the 1st line of
Regards
Rüdiger
