*) mod_http2: new configuration directive: ```H2Padding numbits``` to control

     padding of HTTP/2 payload frames. 'numbits' is a number from 0-8,
     controlling the range of padding bytes added to a frame. The actual number
     added is chosen randomly per frame. This applies to HEADERS, DATA and PUSH_PROMISE
     frames equally. The default continues to be 0, e.g. no padding. [Stefan Eissing] 
  
  *) mod_http2: ripping out all the h2_req_engine internal features now that mod_proxy_http2
     has no more need for it. Optional functions are still declared but no longer implemented.
     While previous mod_proxy_http2 will work with this, it is recommeneded to run the matching
     versions of both modules. [Stefan Eissing]
  
  *) mod_proxy_http2: changed mod_proxy_http2 implementation and fixed several bugs which
     resolve PR63170. The proxy module does now a single h2 request on the (reused)
     connection and returns. [Stefan Eissing]



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1854963 13f79535-47bb-0310-9956-ffa450edef68

1 files changed, 60 insertions, 282 deletions

diff --git a/modules/http2/mod_proxy_http2.c b/modules/http2/mod_proxy_http2.c
index 15418ded58..95336f7576 100644
--- a/modules/http2/mod_proxy_http2.c
+++ b/modules/http2/mod_proxy_http2.c
@@ -47,20 +47,12 @@ AP_DECLARE_MODULE(proxy_http2) = {
 
 /* Optional functions from mod_http2 */
 static int (*is_h2)(conn_rec *c);
-static apr_status_t (*req_engine_push)(const char *name, request_rec *r, 
-                                       http2_req_engine_init *einit);
-static apr_status_t (*req_engine_pull)(h2_req_engine *engine, 
-                                       apr_read_type_e block, 
-                                       int capacity, 
-                                       request_rec **pr);
-static void (*req_engine_done)(h2_req_engine *engine, conn_rec *r_conn,
-                               apr_status_t status);
-                                       
+
 typedef struct h2_proxy_ctx {
+    const char *id;
     conn_rec *master;
     conn_rec *owner;
     apr_pool_t *pool;
-    request_rec *rbase;
     server_rec *server;
     const char *proxy_func;
     char server_portstr[32];
@@ -68,19 +60,16 @@ typedef struct h2_proxy_ctx {
     proxy_worker *worker;
     proxy_server_conf *conf;
     
-    h2_req_engine *engine;
-    const char *engine_id;
-    const char *engine_type;
-    apr_pool_t *engine_pool;    
     apr_size_t req_buffer_size;
-    h2_proxy_fifo *requests;
     int capacity;
     
-    unsigned standalone : 1;
     unsigned is_ssl : 1;
     unsigned flushall : 1;
     
-    apr_status_t r_status;     /* status of our first request work */
+    request_rec *r;            /* the request processed in this ctx */
+    apr_status_t r_status;     /* status of request work */
+    int r_done;                /* request was processed, not necessarily successfully */
+    int r_may_retry;           /* request may be retried */
     h2_proxy_session *session; /* current http2 session against backend */
 } h2_proxy_ctx;
 
@@ -106,16 +95,6 @@ static int h2_proxy_post_config(apr_pool_t *p, apr_pool_t *plog,
                  MOD_HTTP2_VERSION, ngh2? ngh2->version_str : "unknown");
     
     is_h2 = APR_RETRIEVE_OPTIONAL_FN(http2_is_h2);
-    req_engine_push = APR_RETRIEVE_OPTIONAL_FN(http2_req_engine_push);
-    req_engine_pull = APR_RETRIEVE_OPTIONAL_FN(http2_req_engine_pull);
-    req_engine_done = APR_RETRIEVE_OPTIONAL_FN(http2_req_engine_done);
-    
-    /* we need all of them */
-    if (!req_engine_push || !req_engine_pull || !req_engine_done) {
-        req_engine_push = NULL;
-        req_engine_pull = NULL;
-        req_engine_done = NULL;
-    }
     
     return status;
 }
@@ -206,45 +185,6 @@ static int proxy_http2_canon(request_rec *r, char *url)
     return OK;
 }
 
-static void out_consumed(void *baton, conn_rec *c, apr_off_t bytes)
-{
-    h2_proxy_ctx *ctx = baton;
-    
-    if (ctx->session) {
-        h2_proxy_session_update_window(ctx->session, c, bytes);
-    }
-}
-
-static apr_status_t proxy_engine_init(h2_req_engine *engine, 
-                                        const char *id, 
-                                        const char *type,
-                                        apr_pool_t *pool, 
-                                        apr_size_t req_buffer_size,
-                                        request_rec *r,
-                                        http2_output_consumed **pconsumed,
-                                        void **pctx)
-{
-    h2_proxy_ctx *ctx = ap_get_module_config(r->connection->conn_config, 
-                                             &proxy_http2_module);
-    if (!ctx) {
-        ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(03368)
-                      "h2_proxy_session, engine init, no ctx found");
-        return APR_ENOTIMPL;
-    }
-    
-    ctx->pool = pool;
-    ctx->engine = engine;
-    ctx->engine_id = id;
-    ctx->engine_type = type;
-    ctx->engine_pool = pool;
-    ctx->req_buffer_size = req_buffer_size;
-    ctx->capacity = H2MIN(100, h2_proxy_fifo_capacity(ctx->requests));
-    
-    *pconsumed = out_consumed;
-    *pctx = ctx;
-    return APR_SUCCESS;
-}
-
 static apr_status_t add_request(h2_proxy_session *session, request_rec *r)
 {
     h2_proxy_ctx *ctx = session->user_data;
@@ -254,7 +194,7 @@ static apr_status_t add_request(h2_proxy_session *session, request_rec *r)
     url = apr_table_get(r->notes, H2_PROXY_REQ_URL_NOTE);
     apr_table_setn(r->notes, "proxy-source-port", apr_psprintf(r->pool, "%hu",
                    ctx->p_conn->connection->local_addr->port));
-    status = h2_proxy_session_submit(session, url, r, ctx->standalone);
+    status = h2_proxy_session_submit(session, url, r, 1);
     if (status != APR_SUCCESS) {
         ap_log_cerror(APLOG_MARK, APLOG_ERR, status, r->connection, APLOGNO(03351)
                       "pass request body failed to %pI (%s) from %s (%s)",
@@ -268,43 +208,15 @@ static apr_status_t add_request(h2_proxy_session *session, request_rec *r)
 static void request_done(h2_proxy_ctx *ctx, request_rec *r,
                          apr_status_t status, int touched)
 {   
-    const char *task_id = apr_table_get(r->connection->notes, H2_TASK_ID_NOTE);
-
-    ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, r->connection, 
-                  "h2_proxy_session(%s): request done %s, touched=%d",
-                  ctx->engine_id, task_id, touched);
-    if (status != APR_SUCCESS) {
-        if (!touched) {
-            /* untouched request, need rescheduling */
-            status = h2_proxy_fifo_push(ctx->requests, r);
-            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, r->connection, 
-                          APLOGNO(03369)
-                          "h2_proxy_session(%s): rescheduled request %s",
-                          ctx->engine_id, task_id);
-            return;
-        }
-        else {
-            const char *uri;
-            uri = apr_uri_unparse(r->pool, &r->parsed_uri, 0);
-            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, r->connection, 
-                          APLOGNO(03471) "h2_proxy_session(%s): request %s -> %s "
-                          "not complete, cannot repeat", 
-                          ctx->engine_id, task_id, uri);
-        }
-    }
-    
-    if (r == ctx->rbase) {
+    if (r == ctx->r) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, r->connection, 
+                      "h2_proxy_session(%s): request done, touched=%d",
+                      ctx->id, touched);
+        ctx->r_done = 1;
+        if (touched) ctx->r_may_retry = 0;
         ctx->r_status = ((status == APR_SUCCESS)? APR_SUCCESS
                          : HTTP_SERVICE_UNAVAILABLE);
     }
-    
-    if (req_engine_done && ctx->engine) {
-        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, r->connection, 
-                      APLOGNO(03370)
-                      "h2_proxy_session(%s): finished request %s",
-                      ctx->engine_id, task_id);
-        req_engine_done(ctx->engine, r->connection, status);
-    }
 }    
 
 static void session_req_done(h2_proxy_session *session, request_rec *r,
@@ -313,43 +225,15 @@ static void session_req_done(h2_proxy_session *session, request_rec *r,
     request_done(session->user_data, r, status, touched);
 }
 
-static apr_status_t next_request(h2_proxy_ctx *ctx, int before_leave)
-{
-    if (h2_proxy_fifo_count(ctx->requests) > 0) {
-        return APR_SUCCESS;
-    }
-    else if (req_engine_pull && ctx->engine) {
-        apr_status_t status;
-        request_rec *r = NULL;
-        
-        status = req_engine_pull(ctx->engine, before_leave? 
-                                 APR_BLOCK_READ: APR_NONBLOCK_READ, 
-                                 ctx->capacity, &r);
-        if (status == APR_SUCCESS && r) {
-            ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, ctx->owner, 
-                          "h2_proxy_engine(%s): pulled request (%s) %s", 
-                          ctx->engine_id, 
-                          before_leave? "before leave" : "regular", 
-                          r->the_request);
-            h2_proxy_fifo_push(ctx->requests, r);
-        }
-        return APR_STATUS_IS_EAGAIN(status)? APR_SUCCESS : status;
-    }
-    return APR_EOF;
-}
-
-static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) {
+static apr_status_t ctx_run(h2_proxy_ctx *ctx) {
     apr_status_t status = OK;
     int h2_front;
-    request_rec *r;
     
     /* Step Four: Send the Request in a new HTTP/2 stream and
      * loop until we got the response or encounter errors.
      */
-    ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->owner, 
-                  "eng(%s): setup session", ctx->engine_id);
     h2_front = is_h2? is_h2(ctx->owner) : 0;
-    ctx->session = h2_proxy_session_setup(ctx->engine_id, ctx->p_conn, ctx->conf,
+    ctx->session = h2_proxy_session_setup(ctx->id, ctx->p_conn, ctx->conf,
                                           h2_front, 30, 
                                           h2_proxy_log2((int)ctx->req_buffer_size), 
                                           session_req_done);
@@ -360,45 +244,20 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) {
     }
     
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(03373)
-                  "eng(%s): run session %s", ctx->engine_id, ctx->session->id);
+                  "eng(%s): run session %s", ctx->id, ctx->session->id);
     ctx->session->user_data = ctx;
     
-    while (1) {
-        if (ctx->master->aborted) {
-            status = APR_ECONNABORTED;
-            goto out;
-        }
-        
-        if (APR_SUCCESS == h2_proxy_fifo_try_pull(ctx->requests, (void**)&r)) {
-            add_request(ctx->session, r);
-        }
+    ctx->r_done = 0;
+    add_request(ctx->session, ctx->r);
+    
+    while (!ctx->master->aborted && !ctx->r_done) {
+    
         status = h2_proxy_session_process(ctx->session);
-        
-        if (status == APR_SUCCESS) {
-            /* ongoing processing, check if we have room to handle more streams,
-             * maybe the remote side changed their limit */
-            if (ctx->session->remote_max_concurrent > 0
-                && ctx->session->remote_max_concurrent != ctx->capacity) {
-                ctx->capacity = H2MIN((int)ctx->session->remote_max_concurrent, 
-                                      h2_proxy_fifo_capacity(ctx->requests));
-            }
-            /* try to pull more request, if our capacity allows it */
-            if (APR_ECONNABORTED == next_request(ctx, 0)) {
-                status = APR_ECONNABORTED;
-                goto out;
-            }
-            /* If we have no ongoing streams and nothing in our queue, we
-             * terminate processing and return to our caller. */
-            if ((h2_proxy_fifo_count(ctx->requests) == 0) 
-                && h2_proxy_ihash_empty(ctx->session->streams)) {
-                goto out;
-            }
-        }
-        else {
+        if (status != APR_SUCCESS) {
             /* Encountered an error during session processing */
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, 
                           APLOGNO(03375) "eng(%s): end of session %s", 
-                          ctx->engine_id, ctx->session->id);
+                          ctx->id, ctx->session->id);
             /* Any open stream of that session needs to
              * a) be reopened on the new session iff safe to do so
              * b) reported as done (failed) otherwise
@@ -409,12 +268,11 @@ static apr_status_t proxy_engine_run(h2_proxy_ctx *ctx) {
     }
     
 out:
-    if (APR_ECONNABORTED == status) {
+    if (ctx->master->aborted) {
         /* master connection gone */
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, 
-                      APLOGNO(03374) "eng(%s): master connection gone", ctx->engine_id);
-        /* give notice that we're leaving and cancel all ongoing streams. */
-        next_request(ctx, 1); 
+                      APLOGNO(03374) "eng(%s): master connection gone", ctx->id);
+        /* cancel all ongoing requests */
         h2_proxy_session_cancel_all(ctx->session);
         h2_proxy_session_process(ctx->session);
         if (!ctx->master->aborted) {
@@ -427,49 +285,6 @@ out:
     return status;
 }
 
-static apr_status_t push_request_somewhere(h2_proxy_ctx *ctx, request_rec *r)
-{
-    conn_rec *c = ctx->owner;
-    const char *engine_type, *hostname;
-    
-    hostname = (ctx->p_conn->ssl_hostname? 
-                ctx->p_conn->ssl_hostname : ctx->p_conn->hostname);
-    engine_type = apr_psprintf(ctx->pool, "proxy_http2 %s%s", hostname, 
-                               ctx->server_portstr);
-    
-    if (c->master && req_engine_push && r && is_h2 && is_h2(c)) {
-        /* If we are have req_engine capabilities, push the handling of this
-         * request (e.g. slave connection) to a proxy_http2 engine which 
-         * uses the same backend. We may be called to create an engine 
-         * ourself. */
-        if (req_engine_push(engine_type, r, proxy_engine_init) == APR_SUCCESS) {
-            if (ctx->engine == NULL) {
-                /* request has been assigned to an engine in another thread */
-                return SUSPENDED;
-            }
-        }
-    }
-    
-    if (!ctx->engine) {
-        /* No engine was available or has been initialized, handle this
-         * request just by ourself. */
-        ctx->engine_id = apr_psprintf(ctx->pool, "eng-proxy-%ld", c->id);
-        ctx->engine_type = engine_type;
-        ctx->engine_pool = ctx->pool;
-        ctx->req_buffer_size = (32*1024);
-        ctx->standalone = 1;
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, 
-                      "h2_proxy_http2(%ld): setup standalone engine for type %s", 
-                      c->id, engine_type);
-    }
-    else {
-        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, 
-                      "H2: hosting engine %s", ctx->engine_id);
-    }
-
-    return h2_proxy_fifo_push(ctx->requests, r);
-}
-
 static int proxy_http2_handler(request_rec *r, 
                                proxy_worker *worker,
                                proxy_server_conf *conf,
@@ -477,7 +292,7 @@ static int proxy_http2_handler(request_rec *r,
                                const char *proxyname,
                                apr_port_t proxyport)
 {
-    const char *proxy_func;
+    const char *proxy_func, *task_id;
     char *locurl = url, *u;
     apr_size_t slen;
     int is_ssl = 0;
@@ -509,34 +324,35 @@ static int proxy_http2_handler(request_rec *r,
         default:
             return DECLINED;
     }
+
+    task_id = apr_table_get(r->connection->notes, H2_TASK_ID_NOTE);
     
     ctx = apr_pcalloc(r->pool, sizeof(*ctx));
-    ctx->master     = r->connection->master? r->connection->master : r->connection;
-    ctx->owner      = r->connection;
-    ctx->pool       = r->pool;
-    ctx->rbase      = r;
-    ctx->server     = r->server;
+    ctx->master = r->connection->master? r->connection->master : r->connection;
+    ctx->id = task_id? task_id : apr_psprintf(r->pool, "%ld", (long)ctx->master->id);
+    ctx->owner = r->connection;
+    ctx->pool = r->pool;
+    ctx->server = r->server;
     ctx->proxy_func = proxy_func;
-    ctx->is_ssl     = is_ssl;
-    ctx->worker     = worker;
-    ctx->conf       = conf;
-    ctx->flushall   = apr_table_get(r->subprocess_env, "proxy-flushall")? 1 : 0;
-    ctx->r_status   = HTTP_SERVICE_UNAVAILABLE;
-    
-    h2_proxy_fifo_set_create(&ctx->requests, ctx->pool, 100);
+    ctx->is_ssl = is_ssl;
+    ctx->worker = worker;
+    ctx->conf = conf;
+    ctx->flushall = apr_table_get(r->subprocess_env, "proxy-flushall")? 1 : 0;
+    ctx->req_buffer_size = (32*1024);
+    ctx->r = r;
+    ctx->r_status = status = HTTP_SERVICE_UNAVAILABLE;
+    ctx->r_done = 0;
+    ctx->r_may_retry =  1;
     
     ap_set_module_config(ctx->owner->conn_config, &proxy_http2_module, ctx);
 
     /* scheme says, this is for us. */
-    apr_table_setn(ctx->rbase->notes, H2_PROXY_REQ_URL_NOTE, url);
-    ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->rbase, 
+    apr_table_setn(ctx->r->notes, H2_PROXY_REQ_URL_NOTE, url);
+    ap_log_rerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->r, 
                   "H2: serving URL %s", url);
     
 run_connect:    
-    if (ctx->master->aborted) {
-        ctx->r_status = APR_ECONNABORTED;
-        goto cleanup;
-    }
+    if (ctx->master->aborted) goto cleanup;
 
     /* Get a proxy_conn_rec from the worker, might be a new one, might
      * be one still open from another request, or it might fail if the
@@ -551,7 +367,7 @@ run_connect:
     /* Step One: Determine the URL to connect to (might be a proxy),
      * initialize the backend accordingly and determine the server 
      * port string we can expect in responses. */
-    if ((status = ap_proxy_determine_connection(ctx->pool, ctx->rbase, conf, worker, 
+    if ((status = ap_proxy_determine_connection(ctx->pool, ctx->r, conf, worker, 
                                                 ctx->p_conn, &uri, &locurl, 
                                                 proxyname, proxyport, 
                                                 ctx->server_portstr,
@@ -559,17 +375,6 @@ run_connect:
         goto cleanup;
     }
     
-    /* If we are not already hosting an engine, try to push the request 
-     * to an already existing engine or host a new engine here. */
-    if (r && !ctx->engine) {
-        ctx->r_status = push_request_somewhere(ctx, r);
-        r = NULL;
-        if (ctx->r_status == SUSPENDED) {
-            /* request was pushed to another thread, leave processing here */
-            goto cleanup;
-        }
-    }
-    
     /* Step Two: Make the Connection (or check that an already existing
      * socket is still usable). On success, we have a socket connected to
      * backend->hostname. */
@@ -578,19 +383,19 @@ run_connect:
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(03352)
                       "H2: failed to make connection to backend: %s",
                       ctx->p_conn->hostname);
-        goto reconnect;
+        goto cleanup;
     }
     
     /* Step Three: Create conn_rec for the socket we have open now. */
     if (!ctx->p_conn->connection) {
-        status = ap_proxy_connection_create_ex(ctx->proxy_func,
-                                               ctx->p_conn, ctx->rbase);
+        status = ap_proxy_connection_create_ex(ctx->proxy_func, ctx->p_conn, ctx->r);
         if (status != OK) {
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, APLOGNO(03353)
                           "setup new connection: is_ssl=%d %s %s %s", 
                           ctx->p_conn->is_ssl, ctx->p_conn->ssl_hostname, 
                           locurl, ctx->p_conn->hostname);
-            goto reconnect;
+            ctx->r_status = status;
+            goto cleanup;
         }
         
         if (!ctx->p_conn->data && ctx->is_ssl) {
@@ -600,7 +405,7 @@ run_connect:
             apr_table_setn(ctx->p_conn->connection->notes,
                            "proxy-request-alpn-protos", "h2");
             if (ctx->p_conn->ssl_hostname) {
-                ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, ctx->owner, 
+                ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, ctx->owner, 
                               "set SNI to %s for (%s)", 
                               ctx->p_conn->ssl_hostname, 
                               ctx->p_conn->hostname);
@@ -610,33 +415,11 @@ run_connect:
         }
     }
 
-run_session:
-    if (ctx->owner->aborted) {
-        ctx->r_status = APR_ECONNABORTED;
-        goto cleanup;
-    }
-
-    status = proxy_engine_run(ctx);
-    if (status == APR_SUCCESS) {
-        /* session and connection still ok */
-        if (next_request(ctx, 1) == APR_SUCCESS) {
-            /* more requests, run again */
-            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(03376)
-                          "run_session, again");
-            goto run_session;
-        }
-        /* done */
-        ctx->engine = NULL;
-    }
-
-reconnect:
-    if (ctx->master->aborted) {
-        ctx->r_status = APR_ECONNABORTED;
-        goto cleanup;
-    }
+    if (ctx->master->aborted) goto cleanup;
+    status = ctx_run(ctx);
 
-    if (next_request(ctx, 1) == APR_SUCCESS) {
-        /* Still more to do, tear down old conn and start over */
+    if (ctx->r_status != APR_SUCCESS && ctx->r_may_retry && !ctx->master->aborted) {
+        /* Not successfully processed, but may retry, tear down old conn and start over */
         if (ctx->p_conn) {
             ctx->p_conn->close = 1;
 #if AP_MODULE_MAGIC_AT_LEAST(20140207, 2)
@@ -646,12 +429,12 @@ reconnect:
             ctx->p_conn = NULL;
         }
         ++reconnects;
-        if (reconnects < 5 && !ctx->master->aborted) {
+        if (reconnects < 5) {
             goto run_connect;
         } 
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, ctx->owner, APLOGNO(10023)
-                      "giving up after %d reconnects, %d requests todo",
-                      reconnects, h2_proxy_fifo_count(ctx->requests));
+                      "giving up after %d reconnects, request-done=%d",
+                      reconnects, ctx->r_done);
     }
     
 cleanup:
@@ -661,17 +444,12 @@ cleanup:
             ctx->p_conn->close = 1;
         }
 #if AP_MODULE_MAGIC_AT_LEAST(20140207, 2)
-        proxy_run_detach_backend(ctx->rbase, ctx->p_conn);
+        proxy_run_detach_backend(ctx->r, ctx->p_conn);
 #endif
         ap_proxy_release_connection(ctx->proxy_func, ctx->p_conn, ctx->server);
         ctx->p_conn = NULL;
     }
 
-    /* Any requests we still have need to fail */
-    while (APR_SUCCESS == h2_proxy_fifo_try_pull(ctx->requests, (void**)&r)) {
-        request_done(ctx, r, HTTP_SERVICE_UNAVAILABLE, 1);
-    }
-    
     ap_set_module_config(ctx->owner->conn_config, &proxy_http2_module, NULL);
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, ctx->owner, 
                   APLOGNO(03377) "leaving handler");