On 1/17/21 5:21 PM, minf...@apache.org wrote:
> Author: minfrin
> Date: Sun Jan 17 16:21:35 2021
> New Revision: 1885605
>
> URL: http://svn.apache.org/viewvc?rev=1885605&view=rev
> Log:
> Backport to v2.4:
>
> *) mod_proxy_http: handle upgrade/tunneling protocols. BZ 61616 is about
> mod_proxy_connect but there has been wstunnel reports
> on dev@ about that too lately.
> trunk patch: https://svn.apache.org/r1678771
> https://svn.apache.org/r1832348
> https://svn.apache.org/r1869338
> https://svn.apache.org/r1869420
> https://svn.apache.org/r1878367
> https://svn.apache.org/r1877557
> https://svn.apache.org/r1877558
> https://svn.apache.org/r1877646
> https://svn.apache.org/r1877695
> https://svn.apache.org/r1879401
> https://svn.apache.org/r1879402
> https://svn.apache.org/r1880200
> https://svn.apache.org/r1885239
> https://svn.apache.org/r1885240
> https://svn.apache.org/r1885244
> 2.4.x patch:
> http://people.apache.org/~ylavic/patches/2.4.x-mod_proxy_http-upgrade-4on5-v2.patch
> https://github.com/apache/httpd/pull/158
> +1: ylavic, covener, minfrin
> ylavic: All the corresponding trunk changes to mod_proxy_wstunnel (but
> r1885239) have been dropped for this backport proposal, the goal
> being to handle upgrade in mod_proxy_http from now, while
> r1885239
> allows to benefit from the Upgrade improvements done in
> proxy_http
> with existing wstunnel configurations (provided mod_proxy_http
> module is loaded).
>
>
> Modified:
> httpd/httpd/branches/2.4.x/CHANGES
> httpd/httpd/branches/2.4.x/STATUS
> httpd/httpd/branches/2.4.x/include/ap_mmn.h
> httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
> httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.h
> httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_connect.c
> httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_http.c
> httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy_wstunnel.c
> httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c?rev=1885605&r1=1885604&r2=1885605&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c Sun Jan 17 16:21:35
> 2021
i,
> @@ -4119,81 +4168,498 @@ PROXY_DECLARE(apr_status_t) ap_proxy_tra
> const char *name,
> int *sent,
> apr_off_t bsize,
> - int after)
> + int flags)
> {
> apr_status_t rv;
> + int flush_each = 0;
> + unsigned int num_reads = 0;
> #ifdef DEBUGGING
> apr_off_t len;
> #endif
>
> - do {
> + /*
> + * Compat: since FLUSH_EACH is default (and zero) for legacy reasons, we
> + * pretend it's no FLUSH_AFTER nor YIELD_PENDING flags, the latter
> because
> + * flushing would defeat the purpose of checking for pending data (hence
> + * determine whether or not the output chain/stack is full for stopping).
> + */
> + if (!(flags & (AP_PROXY_TRANSFER_FLUSH_AFTER |
> + AP_PROXY_TRANSFER_YIELD_PENDING))) {
> + flush_each = 1;
> + }
> +
> + for (;;) {
> apr_brigade_cleanup(bb_i);
> rv = ap_get_brigade(c_i->input_filters, bb_i, AP_MODE_READBYTES,
> APR_NONBLOCK_READ, bsize);
> - if (rv == APR_SUCCESS) {
> - if (c_o->aborted) {
> - return APR_EPIPE;
> - }
> - if (APR_BRIGADE_EMPTY(bb_i)) {
> - break;
> + if (rv != APR_SUCCESS) {
> + if (!APR_STATUS_IS_EAGAIN(rv) && !APR_STATUS_IS_EOF(rv)) {
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv, r, APLOGNO(03308)
> + "ap_proxy_transfer_between_connections: "
> + "error on %s - ap_get_brigade",
> + name);
> + if (rv == APR_INCOMPLETE) {
> + /* Don't return APR_INCOMPLETE, it'd mean "should yield"
> + * for the caller, while it means "incomplete body" here
> + * from ap_http_filter(), which is an error.
> + */
> + rv = APR_EGENERAL;
> + }
> }
> + break;
> + }
> +
> + if (c_o->aborted) {
> + apr_brigade_cleanup(bb_i);
> + flags &= ~AP_PROXY_TRANSFER_FLUSH_AFTER;
> + rv = APR_EPIPE;
> + break;
> + }
> + if (APR_BRIGADE_EMPTY(bb_i)) {
> + break;
> + }
> #ifdef DEBUGGING
> - len = -1;
> - apr_brigade_length(bb_i, 0, &len);
> - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03306)
> - "ap_proxy_transfer_between_connections: "
> - "read %" APR_OFF_T_FMT
> - " bytes from %s", len, name);
> + len = -1;
> + apr_brigade_length(bb_i, 0, &len);
> + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03306)
> + "ap_proxy_transfer_between_connections: "
> + "read %" APR_OFF_T_FMT
> + " bytes from %s", len, name);
> #endif
> - if (sent) {
> - *sent = 1;
> - }
> - ap_proxy_buckets_lifetime_transform(r, bb_i, bb_o);
> - if (!after) {
> - apr_bucket *b;
> + if (sent) {
> + *sent = 1;
> + }
> + ap_proxy_buckets_lifetime_transform(r, bb_i, bb_o);
> + if (flush_each) {
> + apr_bucket *b;
> + /*
> + * Do not use ap_fflush here since this would cause the flush
> + * bucket to be sent in a separate brigade afterwards which
> + * causes some filters to set aside the buckets from the first
> + * brigade and process them when FLUSH arrives in the second
> + * brigade. As set asides of our transformed buckets involve
> + * memory copying we try to avoid this. If we have the flush
> + * bucket in the first brigade they directly process the
> + * buckets without setting them aside.
> + */
> + b = apr_bucket_flush_create(bb_o->bucket_alloc);
> + APR_BRIGADE_INSERT_TAIL(bb_o, b);
> + }
> + rv = ap_pass_brigade(c_o->output_filters, bb_o);
> + apr_brigade_cleanup(bb_o);
> + if (rv != APR_SUCCESS) {
> + ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(03307)
> + "ap_proxy_transfer_between_connections: "
> + "error on %s - ap_pass_brigade",
> + name);
> + flags &= ~AP_PROXY_TRANSFER_FLUSH_AFTER;
> + break;
> + }
>
> - /*
> - * Do not use ap_fflush here since this would cause the flush
> - * bucket to be sent in a separate brigade afterwards which
> - * causes some filters to set aside the buckets from the
> first
> - * brigade and process them when the flush arrives in the
> second
> - * brigade. As set asides of our transformed buckets involve
> - * memory copying we try to avoid this. If we have the flush
> - * bucket in the first brigade they directly process the
> - * buckets without setting them aside.
> - */
> - b = apr_bucket_flush_create(bb_o->bucket_alloc);
> - APR_BRIGADE_INSERT_TAIL(bb_o, b);
> + /* Yield if the output filters stack is full? This is to avoid
> + * blocking and give the caller a chance to POLLOUT async.
> + */
> + if (flags & AP_PROXY_TRANSFER_YIELD_PENDING) {
> + int rc = OK;
> +
> + if (!ap_filter_should_yield(c_o->output_filters)) {
> + rc = ap_filter_output_pending(c_o);
I am confused here: !ap_filter_should_yield(c_o->output_filters) means there is
no pending output.
Why should we try to send it then? Shouldn't it be the other way round?
> }
> - rv = ap_pass_brigade(c_o->output_filters, bb_o);
> - if (rv != APR_SUCCESS) {
> - ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(03307)
> + if (rc == OK) {
> + ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
> "ap_proxy_transfer_between_connections: "
> - "error on %s - ap_pass_brigade",
> - name);
> + "yield (output pending)");
> + rv = APR_INCOMPLETE;
> + break;
> + }
> + if (rc != DECLINED) {
> + rv = AP_FILTER_ERROR;
> + break;
Regards
Rüdiger
