v1.10.12 from apache svn

ChangeLog

@@ -1,3 +1,8 @@
+v1.10.12
+--------------------------------------------------------------------------------
+  * Reverting 1.10.11 change since showed no effect on issue #120
+  * Fixes a race condition on request body handling.
+
 v1.10.11
 --------------------------------------------------------------------------------
   * DoS flow control protection is less agressive as long as active tasks stay

configure.ac

@@ -14,7 +14,7 @@
 #
 
 AC_PREREQ([2.69])
-AC_INIT([mod_http2], [1.10.11], [[email protected]])
+AC_INIT([mod_http2], [1.10.12], [[email protected]])
 
 LT_PREREQ([2.2.6])
 LT_INIT()

mod_http2/h2_bucket_beam.c

@@ -210,16 +210,7 @@ static apr_status_t mutex_enter(void *ctx, h2_beam_lock *pbl)
 
 static apr_status_t enter_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
 {
-    h2_beam_mutex_enter *enter = beam->m_enter;
-    if (enter) {
-        void *ctx = beam->m_ctx;
-        if (ctx) {
-            return enter(ctx, pbl);
-        }
-    }
-    pbl->mutex = NULL;
-    pbl->leave = NULL;
-    return APR_SUCCESS;
+    return mutex_enter(beam, pbl);
 }
 
 static void leave_yellow(h2_bucket_beam *beam, h2_beam_lock *pbl)
@@ -568,13 +559,12 @@ static apr_status_t beam_cleanup(void *data)
 {
     h2_bucket_beam *beam = data;
     apr_status_t status = APR_SUCCESS;
-    int safe_send = !beam->m_enter || (beam->owner == H2_BEAM_OWNER_SEND);
-    int safe_recv = !beam->m_enter || (beam->owner == H2_BEAM_OWNER_RECV);
+    int safe_send = (beam->owner == H2_BEAM_OWNER_SEND);
+    int safe_recv = (beam->owner == H2_BEAM_OWNER_RECV);
 
     /* 
      * Owner of the beam is going away, depending on which side it owns,
-     * cleanup strategies will differ with multi-thread protection
-     * still in place (beam->m_enter).
+     * cleanup strategies will differ.
      *
      * In general, receiver holds references to memory from sender. 
      * Clean up receiver first, if safe, then cleanup sender, if safe.
@@ -680,29 +670,6 @@ apr_size_t h2_beam_buffer_size_get(h2_bucket_beam *beam)
     return buffer_size;
 }
 
-void h2_beam_mutex_set(h2_bucket_beam *beam, 
-                       h2_beam_mutex_enter m_enter,
-                       void *m_ctx)
-{
-    h2_beam_lock bl;
-
-    if (enter_yellow(beam, &bl) == APR_SUCCESS) {
-        beam->m_enter = m_enter;
-        beam->m_ctx   = m_ctx;
-        leave_yellow(beam, &bl);
-    }
-}
-
-void h2_beam_mutex_enable(h2_bucket_beam *beam)
-{
-    h2_beam_mutex_set(beam, mutex_enter, beam);
-}
-
-void h2_beam_mutex_disable(h2_bucket_beam *beam)
-{
-    h2_beam_mutex_set(beam, NULL, NULL);
-}
-
 void h2_beam_timeout_set(h2_bucket_beam *beam, apr_interval_time_t timeout)
 {
     h2_beam_lock bl;

mod_http2/h2_bucket_beam.h

@@ -64,13 +64,11 @@ typedef struct {
  * via the h2_beam_send(). It gives the beam to the green thread which then
  * can receive buckets into its own brigade via h2_beam_receive().
  *
- * Sending and receiving can happen concurrently, if a thread mutex is set
- * for the beam, see h2_beam_mutex_set.
+ * Sending and receiving can happen concurrently.
  *
  * The beam can limit the amount of data it accepts via the buffer_size. This
- * can also be adjusted during its lifetime. When the beam not only gets a 
- * mutex but als a condition variable (in h2_beam_mutex_set()), sends and
- * receives can be done blocking. A timeout can be set for such blocks.
+ * can also be adjusted during its lifetime. Sends and receives can be done blocking. 
+ * A timeout can be set for such blocks.
  *
  * Care needs to be taken when terminating the beam. The beam registers at
  * the pool it was created with and will cleanup after itself. However, if
@@ -191,8 +189,6 @@ struct h2_bucket_beam {
 
     struct apr_thread_mutex_t *lock;
     struct apr_thread_cond_t *change;
-    void *m_ctx;
-    h2_beam_mutex_enter *m_enter;
 
     apr_off_t cons_bytes_reported;    /* amount of bytes reported as consumed */
     h2_beam_ev_callback *cons_ev_cb;
@@ -315,13 +311,6 @@ int h2_beam_is_closed(h2_bucket_beam *beam);
  */
 apr_status_t h2_beam_wait_empty(h2_bucket_beam *beam, apr_read_type_e block);
 
-void h2_beam_mutex_set(h2_bucket_beam *beam, 
-                       h2_beam_mutex_enter m_enter,
-                       void *m_ctx);
-
-void h2_beam_mutex_enable(h2_bucket_beam *beam);
-void h2_beam_mutex_disable(h2_bucket_beam *beam);
-
 /** 
  * Set/get the timeout for blocking read/write operations. Only works
  * if a mutex has been set for the beam.

mod_http2/h2_mplx.c

@@ -543,9 +543,6 @@ static apr_status_t out_open(h2_mplx *m, int stream_id, h2_bucket_beam *beam)
         h2_beam_on_file_beam(stream->output, h2_beam_no_files, NULL);
     }
 
-    /* time to protect the beam against multi-threaded use */
-    h2_beam_mutex_enable(stream->output);
-
     /* we might see some file buckets in the output, see
      * if we have enough handles reserved. */
     check_data_for(m, stream, 0);
@@ -853,10 +850,6 @@ static void task_done(h2_mplx *m, h2_task *task, h2_req_engine *ngn)
                           H2_STRM_MSG(stream, "task_done, stream open")); 
             if (stream->input) {
                 h2_beam_leave(stream->input);
-                h2_beam_mutex_disable(stream->input);
-            }
-            if (stream->output) {
-                h2_beam_mutex_disable(stream->output);
             }
 
             /* more data will not arrive, resume the stream */
@@ -869,10 +862,6 @@ static void task_done(h2_mplx *m, h2_task *task, h2_req_engine *ngn)
                       H2_STRM_MSG(stream, "task_done, in hold"));
         if (stream->input) {
             h2_beam_leave(stream->input);
-            h2_beam_mutex_disable(stream->input);
-        }
-        if (stream->output) {
-            h2_beam_mutex_disable(stream->output);
         }
         stream_joined(m, stream);
     }
@@ -969,29 +958,19 @@ static h2_stream *get_timed_out_busy_stream(h2_mplx *m)
 static apr_status_t unschedule_slow_tasks(h2_mplx *m) 
 {
     h2_stream *stream;
-    int n, amax;
+    int n;
 
     /* Try to get rid of streams that occupy workers. Look for safe requests
      * that are repeatable. If none found, fail the connection.
-     *
-     * see: https://github.com/icing/mod_h2/issues/120
-     * Enforcing m->limit_active (which can go as low as 2) was too 
-     * aggressive for media streaming. Players needed to re-open streams/connections
-     * continously as they were reading large files very slowly.
      */
-    amax = m->max_active/2;
-    if (m->limit_active > amax) {
-        amax = m->limit_active; 
-    }
-    n = (m->tasks_active - amax - (int)h2_ihash_count(m->sredo));
-
+    n = (m->tasks_active - m->limit_active - (int)h2_ihash_count(m->sredo));
     while (n > 0 && (stream = get_latest_repeatable_unsubmitted_stream(m))) {
         h2_task_rst(stream->task, H2_ERR_CANCEL);
         h2_ihash_add(m->sredo, stream);
         --n;
     }
 
-    if ((m->tasks_active - h2_ihash_count(m->sredo)) > amax) {
+    if ((m->tasks_active - h2_ihash_count(m->sredo)) > m->limit_active) {
         h2_stream *stream = get_timed_out_busy_stream(m);
         if (stream) {
             /* Too many busy workers, unable to cancel enough streams

mod_http2/h2_task.c

@@ -613,10 +613,6 @@ apr_status_t h2_task_do(h2_task *task, apr_thread_t *thread, int worker_id)
     h2_ctx_create_for(c, task);
     apr_table_setn(c->notes, H2_TASK_ID_NOTE, task->id);
 
-    if (task->input.beam) {
-        h2_beam_mutex_enable(task->input.beam);
-    }
-
     h2_slave_run_pre_connection(c, ap_get_conn_socket(c));            
 
     task->input.bb = apr_brigade_create(task->pool, c->bucket_alloc);

mod_http2/h2_util.c

@@ -112,6 +112,8 @@ void h2_util_camel_case_header(char *s, size_t len)
     }
 }
 
+/* base64 url encoding ****************************************************************************/
+
 static const int BASE64URL_UINT6[] = {
 /*   0   1   2   3   4   5   6   7   8   9   a   b   c   d   e   f        */
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, /*  0 */
@@ -174,13 +176,15 @@ apr_size_t h2_util_base64url_decode(const char **decoded, const char *encoded,
             n = ((BASE64URL_UINT6[ e[mlen+0] ] << 18) +
                  (BASE64URL_UINT6[ e[mlen+1] ] << 12));
             *d++ = n >> 16;
+            remain = 1;
             break;
         case 3:
             n = ((BASE64URL_UINT6[ e[mlen+0] ] << 18) +
                  (BASE64URL_UINT6[ e[mlen+1] ] << 12) +
                  (BASE64URL_UINT6[ e[mlen+2] ] << 6));
             *d++ = n >> 16;
             *d++ = n >> 8 & 0xffu;
+            remain = 2;
             break;
         default: /* do nothing */
             break;
@@ -189,78 +193,35 @@ apr_size_t h2_util_base64url_decode(const char **decoded, const char *encoded,
 }
 
 const char *h2_util_base64url_encode(const char *data, 
-                                     apr_size_t len, apr_pool_t *pool)
+                                     apr_size_t dlen, apr_pool_t *pool)
 {
-    apr_size_t mlen = ((len+2)/3)*3;
-    apr_size_t slen = (mlen/3)*4;
-    apr_size_t i;
+    long i, len = (int)dlen;
+    apr_size_t slen = ((dlen+2)/3)*4 + 1; /* 0 terminated */
     const unsigned char *udata = (const unsigned char*)data;
-    char *enc, *p = apr_pcalloc(pool, slen+1); /* 0 terminated */
+    char *enc, *p = apr_pcalloc(pool, slen);
 
     enc = p;
-    for (i = 0; i < mlen; i+= 3) {
+    for (i = 0; i < len-2; i+= 3) {
         *p++ = BASE64URL_CHARS[ (udata[i] >> 2) & 0x3fu ];
-        *p++ = BASE64URL_CHARS[ ((udata[i] << 4) + 
-                                 ((i+1 < len)? (udata[i+1] >> 4) : 0)) & 0x3fu ];
-        *p++ = BASE64URL_CHARS[ ((udata[i+1] << 2) + 
-                                 ((i+2 < len)? (udata[i+2] >> 6) : 0)) & 0x3fu ];
-        if (i+2 < len) {
-            *p++ = BASE64URL_CHARS[ udata[i+2] & 0x3fu ];
-        }
+        *p++ = BASE64URL_CHARS[ ((udata[i] << 4) + (udata[i+1] >> 4)) & 0x3fu ];
+        *p++ = BASE64URL_CHARS[ ((udata[i+1] << 2) + (udata[i+2] >> 6)) & 0x3fu ];
+        *p++ = BASE64URL_CHARS[ udata[i+2] & 0x3fu ];
     }
 
-    return enc;
-}
-
-int h2_util_contains_token(apr_pool_t *pool, const char *s, const char *token)
-{
-    char *c;
-    if (s) {
-        if (!apr_strnatcasecmp(s, token)) {   /* the simple life */
-            return 1;
-        }
-
-        for (c = ap_get_token(pool, &s, 0); c && *c;
-             c = *s? ap_get_token(pool, &s, 0) : NULL) {
-            if (!apr_strnatcasecmp(c, token)) { /* seeing the token? */
-                return 1;
-            }
-            while (*s++ == ';') {            /* skip parameters */
-                ap_get_token(pool, &s, 0);
-            }
-            if (*s++ != ',') {               /* need comma separation */
-                return 0;
-            }
+    if (i < len) {
+        *p++ = BASE64URL_CHARS[ (udata[i] >> 2) & 0x3fu ];
+        if (i == (len - 1)) {
+            *p++ = BASE64URL_CHARS[ (udata[i] << 4) & 0x3fu ];
         }
-    }
-    return 0;
-}
-
-const char *h2_util_first_token_match(apr_pool_t *pool, const char *s, 
-                                      const char *tokens[], apr_size_t len)
-{
-    char *c;
-    apr_size_t i;
-    if (s && *s) {
-        for (c = ap_get_token(pool, &s, 0); c && *c;
-             c = *s? ap_get_token(pool, &s, 0) : NULL) {
-            for (i = 0; i < len; ++i) {
-                if (!apr_strnatcasecmp(c, tokens[i])) {
-                    return tokens[i];
-                }
-            }
-            while (*s++ == ';') {            /* skip parameters */
-                ap_get_token(pool, &s, 0);
-            }
-            if (*s++ != ',') {               /* need comma separation */
-                return 0;
-            }
+        else {
+            *p++ = BASE64URL_CHARS[ ((udata[i] << 4) + (udata[i+1] >> 4)) & 0x3fu ];
+            *p++ = BASE64URL_CHARS[ (udata[i+1] << 2) & 0x3fu ];
         }
     }
-    return NULL;
+    *p++ = '\0';
+    return enc;
 }
 
-
 /*******************************************************************************
  * ihash - hash for structs with int identifier
  ******************************************************************************/

mod_http2/h2_util.h

@@ -337,15 +337,6 @@ unsigned char h2_log2(int n);
  */
 apr_size_t h2_util_table_bytes(apr_table_t *t, apr_size_t pair_extra);
 
-/**
- * Return != 0 iff the string s contains the token, as specified in
- * HTTP header syntax, rfc7230.
- */
-int h2_util_contains_token(apr_pool_t *pool, const char *s, const char *token);
-
-const char *h2_util_first_token_match(apr_pool_t *pool, const char *s, 
-                                      const char *tokens[], apr_size_t len);
-
 /** Match a header value against a string constance, case insensitive */
 #define H2_HD_MATCH_LIT(l, name, nlen)  \
     ((nlen == sizeof(l) - 1) && !apr_strnatcasecmp(l, name))

mod_http2/h2_version.h

@@ -26,15 +26,15 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.10.11-git"
+#define MOD_HTTP2_VERSION "1.10.12-git"
 
 /**
  * @macro
  * Numerical representation of the version number of the http2 module
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x010a0b
+#define MOD_HTTP2_VERSION_NUM 0x010a0c
 
 
 #endif /* mod_h2_h2_version_h */

mod_http2/h2_workers.c

@@ -98,6 +98,8 @@ static apr_status_t activate_slot(h2_workers *workers, h2_slot *slot)
         }
     }
 
+    ap_log_error(APLOG_MARK, APLOG_TRACE2, 0, workers->s,
+                 "h2_workers: new thread for slot %d", slot->id); 
     /* thread will either immediately start work or add itself
      * to the idle queue */
     apr_thread_create(&slot->thread, workers->thread_attr, slot_run, slot,