On the trunk:

mod_http2: only when 'HttpProtocolOptions Unsafe' is configured, will
     control characters in response headers or trailers be forwarded to the
     client. Otherwise, in the default configuration, a request will eiher 
     fail with status 500 or the stream will be reset by a RST_STREAM frame. 



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

CHANGES

@@ -1,6 +1,12 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_http2: only when 'HttpProtocolOptions Unsafe' is configured, will
+     control characters in response headers or trailers be forwarded to the
+     client. Otherwise, in the default configuration, a request will eiher 
+     fail with status 500 or the stream will be reset by a RST_STREAM frame. 
+     [Stefan Eissing]
+     
   *) core: Disallow multiple Listen on the same IP:port when listener buckets
      are configured (ListenCoresBucketsRatio > 0), consistently with the single
      bucket case (default), thus avoiding the leak of the corresponding socket

modules/http2/h2.h

@@ -154,5 +154,7 @@ typedef int h2_stream_pri_cmp(int stream_id1, int stream_id2, void *ctx);
 
 #define H2_TASK_ID_NOTE         "http2-task-id"
 #define H2_FILTER_DEBUG_NOTE    "http2-debug"
+#define H2_HDR_CONFORMANCE      "http2-hdr-conformance"
+#define H2_HDR_CONFORMANCE_UNSAFE      "unsafe"
 
 #endif /* defined(__mod_h2__h2__) */

modules/http2/h2_headers.c

@@ -32,6 +32,12 @@
 #include "h2_headers.h"
 
 
+static int is_unsafe(server_rec *s) 
+{
+    core_server_config *conf = ap_get_core_module_config(s->module_config);
+    return (conf->http_conformance == AP_HTTP_CONFORMANCE_UNSAFE);
+}
+
 typedef struct {
     apr_bucket_refcount refcount;
     h2_headers *headers;
@@ -132,9 +138,19 @@ h2_headers *h2_headers_rcreate(request_rec *r, int status,
             headers->status = H2_ERR_HTTP_1_1_REQUIRED;
         }
     }
+    if (is_unsafe(r->server)) {
+        apr_table_setn(headers->notes, H2_HDR_CONFORMANCE, 
+                       H2_HDR_CONFORMANCE_UNSAFE);
+    }
     return headers;
 }
 
+h2_headers *h2_headers_copy(apr_pool_t *pool, h2_headers *h)
+{
+    return h2_headers_create(h->status, apr_table_copy(pool, h->headers), 
+                             apr_table_copy(pool, h->notes), pool);
+}
+
 h2_headers *h2_headers_die(apr_status_t type,
                              const h2_request *req, apr_pool_t *pool)
 {

modules/http2/h2_headers.h

@@ -55,6 +55,12 @@ h2_headers *h2_headers_create(int status, apr_table_t *header,
 h2_headers *h2_headers_rcreate(request_rec *r, int status, 
                                  apr_table_t *header, apr_pool_t *pool);
 
+/**
+ * Clone the headers into another pool. This will not copy any
+ * header strings.
+ */
+h2_headers *h2_headers_copy(apr_pool_t *pool, h2_headers *h);
+
 /**
  * Create the headers for the given error.
  * @param stream_id id of the stream to create the headers for

modules/http2/h2_request.c

@@ -39,16 +39,15 @@
 typedef struct {
     apr_table_t *headers;
     apr_pool_t *pool;
+    apr_status_t status;
 } h1_ctx;
 
 static int set_h1_header(void *ctx, const char *key, const char *value)
 {
     h1_ctx *x = ctx;
-    size_t klen = strlen(key);
+    x->status = h2_req_add_header(x->headers, x->pool, key, strlen(key), 
-    if (!h2_req_ignore_header(key, klen)) {
+                                  value, strlen(value));
-        h2_headers_add_h1(x->headers, x->pool, key, klen, value, strlen(value));
+    return (x->status == APR_SUCCESS)? 1 : 0;
-    }
-    return 1;
 }
 
 apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool, 
@@ -90,10 +89,11 @@ apr_status_t h2_request_rcreate(h2_request **preq, apr_pool_t *pool,
 
     x.pool = pool;
     x.headers = req->headers;
+    x.status = APR_SUCCESS;
     apr_table_do(set_h1_header, &x, r->headers_in, NULL);
     
     *preq = req;
-    return APR_SUCCESS;
+    return x.status;
 }
 
 apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool, 
@@ -143,7 +143,7 @@ apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool,
     }
     else {
         /* non-pseudo header, append to work bucket of stream */
-        status = h2_headers_add_h1(req->headers, pool, name, nlen, value, vlen);
+        status = h2_req_add_header(req->headers, pool, name, nlen, value, vlen);
     }
     
     return status;

modules/http2/h2_session.c

@@ -1080,13 +1080,16 @@ struct h2_stream *h2_session_push(h2_session *session, h2_stream *is,
 {
     h2_stream *stream;
     h2_ngheader *ngh;
-    int nid;
+    apr_status_t status;
+    int nid = 0;
     
-    ngh = h2_util_ngheader_make_req(is->pool, push->req);
+    status = h2_req_create_ngheader(&ngh, is->pool, push->req);
-    nid = nghttp2_submit_push_promise(session->ngh2, 0, is->id, 
+    if (status == APR_SUCCESS) {
-                                      ngh->nv, ngh->nvlen, NULL);
+        nid = nghttp2_submit_push_promise(session->ngh2, 0, is->id, 
-    if (nid <= 0) {
+                                          ngh->nv, ngh->nvlen, NULL);
-        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, 
+    }
+    if (status != APR_SUCCESS || nid <= 0) {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c, 
                       H2_STRM_LOG(APLOGNO(03075), is, 
                       "submitting push promise fail: %s"), nghttp2_strerror(nid));
         return NULL;
@@ -1280,16 +1283,25 @@ static apr_status_t on_stream_headers(h2_session *session, h2_stream *stream,
     else if (stream->has_response) {
         h2_ngheader *nh;
         
-        nh = h2_util_ngheader_make(stream->pool, headers->headers);
+        status = h2_res_create_ngtrailer(&nh, stream->pool, headers);
-        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, 
+        
-                      H2_STRM_LOG(APLOGNO(03072), stream, "submit %d trailers"), (int)nh->nvlen);
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c, 
-        rv = nghttp2_submit_trailer(session->ngh2, stream->id, nh->nv, nh->nvlen);
+                      H2_STRM_LOG(APLOGNO(03072), stream, "submit %d trailers"), 
+                      (int)nh->nvlen);
+        if (status == APR_SUCCESS) {
+            rv = nghttp2_submit_trailer(session->ngh2, stream->id, 
+                                        nh->nv, nh->nvlen);
+        }
+        else {
+            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c,
+                          H2_STRM_LOG(APLOGNO(), stream, "invalid trailers"));
+            h2_stream_rst(stream, NGHTTP2_PROTOCOL_ERROR);
+        }
         goto leave;
     }
     else {
         nghttp2_data_provider provider, *pprovider = NULL;
         h2_ngheader *ngh;
-        apr_table_t *hout;
         const char *note;
         
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, 
@@ -1335,17 +1347,16 @@ static apr_status_t on_stream_headers(h2_session *session, h2_stream *stream,
         }
         h2_session_set_prio(session, stream, stream->pref_priority);
         
-        hout = headers->headers;
         note = apr_table_get(headers->notes, H2_FILTER_DEBUG_NOTE);
         if (note && !strcmp("on", note)) {
             int32_t connFlowIn, connFlowOut;
 
             connFlowIn = nghttp2_session_get_effective_local_window_size(session->ngh2); 
             connFlowOut = nghttp2_session_get_remote_window_size(session->ngh2);
-            hout = apr_table_clone(stream->pool, hout);
+            headers = h2_headers_copy(stream->pool, headers);
-            apr_table_setn(hout, "conn-flow-in", 
+            apr_table_setn(headers->headers, "conn-flow-in", 
                            apr_itoa(stream->pool, connFlowIn));
-            apr_table_setn(hout, "conn-flow-out", 
+            apr_table_setn(headers->headers, "conn-flow-out", 
                            apr_itoa(stream->pool, connFlowOut));
         }
         
@@ -1357,17 +1368,24 @@ static apr_status_t on_stream_headers(h2_session *session, h2_stream *stream,
             goto leave;
         }
         
-        ngh = h2_util_ngheader_make_res(stream->pool, headers->status, hout);
+        status = h2_res_create_ngheader(&ngh, stream->pool, headers);
-        rv = nghttp2_submit_response(session->ngh2, stream->id,
+        if (status == APR_SUCCESS) {
-                                     ngh->nv, ngh->nvlen, pprovider);
+            rv = nghttp2_submit_response(session->ngh2, stream->id,
-        stream->has_response = h2_headers_are_response(headers);
+                                         ngh->nv, ngh->nvlen, pprovider);
-        session->have_written = 1;
+            stream->has_response = h2_headers_are_response(headers);
-        
+            session->have_written = 1;
-        if (stream->initiated_on) {
+            
-            ++session->pushes_submitted;
+            if (stream->initiated_on) {
+                ++session->pushes_submitted;
+            }
+            else {
+                ++session->responses_submitted;
+            }
         }
         else {
-            ++session->responses_submitted;
+            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, session->c,
+                          H2_STRM_LOG(APLOGNO(), stream, "invalid response"));
+            h2_stream_rst(stream, NGHTTP2_PROTOCOL_ERROR);
         }
     }
     

modules/http2/h2_util.c

@@ -1382,89 +1382,150 @@ static int count_header(void *ctx, const char *key, const char *value)
     return 1;
 }
 
-#define NV_ADD_LIT_CS(nv, k, v)     add_header(nv, k, sizeof(k) - 1, v, strlen(v))
+static const char *inv_field_name_chr(const char *token)
-#define NV_ADD_CS_CS(nv, k, v)      add_header(nv, k, strlen(k), v, strlen(v))
-
-static int add_header(h2_ngheader *ngh, 
-                      const char *key, size_t key_len,
-                      const char *value, size_t val_len)
 {
-    nghttp2_nv *nv = &ngh->nv[ngh->nvlen++];
+    const char *p = ap_scan_http_token(token);
-    
+    if (p == token && *p == ':') {
+        p = ap_scan_http_token(++p);
+    }
+    return (p && *p)? p : NULL;
+}
+
+static const char *inv_field_value_chr(const char *token)
+{
+    const char *p = ap_scan_http_field_content(token);
+    return (p && *p)? p : NULL;
+}
+
+typedef struct ngh_ctx {
+    apr_pool_t *p;
+    int unsafe;
+    h2_ngheader *ngh;
+    apr_status_t status;
+} ngh_ctx;
+
+static int add_header(ngh_ctx *ctx, const char *key, const char *value)
+{
+    nghttp2_nv *nv = &(ctx->ngh)->nv[(ctx->ngh)->nvlen++];
+    const char *p;
+
+    if (!ctx->unsafe) {
+        if ((p = inv_field_name_chr(key))) {
+            ap_log_perror(APLOG_MARK, APLOG_TRACE1, APR_EINVAL, ctx->p,
+                          "h2_request: head field '%s: %s' has invalid char %s", 
+                          key, value, p);
+            ctx->status = APR_EINVAL;
+            return 0;
+        }
+        if ((p = inv_field_value_chr(value))) {
+            ap_log_perror(APLOG_MARK, APLOG_TRACE1, APR_EINVAL, ctx->p,
+                          "h2_request: head field '%s: %s' has invalid char %s", 
+                          key, value, p);
+            ctx->status = APR_EINVAL;
+            return 0;
+        }
+    }
     nv->name = (uint8_t*)key;
-    nv->namelen = key_len;
+    nv->namelen = strlen(key);
     nv->value = (uint8_t*)value;
-    nv->valuelen = val_len;
+    nv->valuelen = strlen(value);
+    
     return 1;
 }
 
 static int add_table_header(void *ctx, const char *key, const char *value)
 {
     if (!h2_util_ignore_header(key)) {
-        add_header(ctx, key, strlen(key), value, strlen(value));
+        add_header(ctx, key, value);
     }
     return 1;
 }
 
-
+static apr_status_t ngheader_create(h2_ngheader **ph, apr_pool_t *p, 
-h2_ngheader *h2_util_ngheader_make(apr_pool_t *p, apr_table_t *header)
+                                    int unsafe, size_t key_count, 
+                                    const char *keys[], const char *values[],
+                                    apr_table_t *headers)
 {
-    h2_ngheader *ngh;
+    ngh_ctx ctx;
-    size_t n;
+    size_t n, i;
     
-    n = 0;
+    ctx.p = p;
-    apr_table_do(count_header, &n, header, NULL);
+    ctx.unsafe = unsafe;
     
-    ngh = apr_pcalloc(p, sizeof(h2_ngheader));
+    n = key_count;
-    ngh->nv =  apr_pcalloc(p, n * sizeof(nghttp2_nv));
+    apr_table_do(count_header, &n, headers, NULL);
-    apr_table_do(add_table_header, ngh, header, NULL);
+    
+    *ph = ctx.ngh = apr_pcalloc(p, sizeof(h2_ngheader));
+    if (!ctx.ngh) {
+        return APR_ENOMEM;
+    }
+    
+    ctx.ngh->nv =  apr_pcalloc(p, n * sizeof(nghttp2_nv));
+    if (!ctx.ngh->nv) {
+        return APR_ENOMEM;
+    }
+    
+    ctx.status = APR_SUCCESS;
+    for (i = 0; i < key_count; ++i) {
+        if (!add_header(&ctx, keys[i], values[i])) {
+            return ctx.status;
+        }
+    }
+    
+    apr_table_do(add_table_header, &ctx, headers, NULL);
 
-    return ngh;
+    return ctx.status;
 }
 
-h2_ngheader *h2_util_ngheader_make_res(apr_pool_t *p, 
+static int is_unsafe(h2_headers *h)
-                                       int http_status, 
-                                       apr_table_t *header)
 {
-    h2_ngheader *ngh;
+    const char *v = apr_table_get(h->notes, H2_HDR_CONFORMANCE);
-    size_t n;
+    return (v && !strcmp(v, H2_HDR_CONFORMANCE_UNSAFE));
-    
-    n = 1;
-    apr_table_do(count_header, &n, header, NULL);
-    
-    ngh = apr_pcalloc(p, sizeof(h2_ngheader));
-    ngh->nv =  apr_pcalloc(p, n * sizeof(nghttp2_nv));
-    NV_ADD_LIT_CS(ngh, ":status", apr_psprintf(p, "%d", http_status));
-    apr_table_do(add_table_header, ngh, header, NULL);
-
-    return ngh;
 }
 
-h2_ngheader *h2_util_ngheader_make_req(apr_pool_t *p, 
+apr_status_t h2_res_create_ngtrailer(h2_ngheader **ph, apr_pool_t *p, 
-                                       const struct h2_request *req)
+                                    h2_headers *headers)
+{
+    return ngheader_create(ph, p, is_unsafe(headers), 
+                           0, NULL, NULL, headers->headers);
+}
+                                     
+apr_status_t h2_res_create_ngheader(h2_ngheader **ph, apr_pool_t *p,
+                                    h2_headers *headers) 
+{
+    const char *keys[] = {
+        ":status"
+    };
+    const char *values[] = {
+        apr_psprintf(p, "%d", headers->status)
+    };
+    return ngheader_create(ph, p, is_unsafe(headers),  
+                           H2_ALEN(keys), keys, values, headers->headers);
+}
+
+apr_status_t h2_req_create_ngheader(h2_ngheader **ph, apr_pool_t *p, 
+                                    const struct h2_request *req)
 {
     
-    h2_ngheader *ngh;
+    const char *keys[] = {
-    size_t n;
+        ":scheme", 
+        ":authority", 
+        ":path", 
+        ":method", 
+    };
+    const char *values[] = {
+        req->scheme,
+        req->authority, 
+        req->path, 
+        req->method, 
+    };
     
-    ap_assert(req);
     ap_assert(req->scheme);
     ap_assert(req->authority);
     ap_assert(req->path);
     ap_assert(req->method);
 
-    n = 4;
+    return ngheader_create(ph, p, 0, H2_ALEN(keys), keys, values, req->headers);
-    apr_table_do(count_header, &n, req->headers, NULL);
-    
-    ngh = apr_pcalloc(p, sizeof(h2_ngheader));
-    ngh->nv =  apr_pcalloc(p, n * sizeof(nghttp2_nv));
-    NV_ADD_LIT_CS(ngh, ":scheme", req->scheme);
-    NV_ADD_LIT_CS(ngh, ":authority", req->authority);
-    NV_ADD_LIT_CS(ngh, ":path", req->path);
-    NV_ADD_LIT_CS(ngh, ":method", req->method);
-    apr_table_do(add_table_header, ngh, req->headers, NULL);
-
-    return ngh;
 }
 
 /*******************************************************************************
@@ -1481,7 +1542,6 @@ typedef struct {
 #define H2_LIT_ARGS(a)      (a),H2_ALEN(a)
 
 static literal IgnoredRequestHeaders[] = {
-/*H2_DEF_LITERAL("expect"),*/
     H2_DEF_LITERAL("upgrade"),
     H2_DEF_LITERAL("connection"),
     H2_DEF_LITERAL("keep-alive"),
@@ -1547,7 +1607,7 @@ int h2_res_ignore_trailer(const char *name, size_t len)
     return ignore_header(H2_LIT_ARGS(IgnoredResponseTrailers), name, len);
 }
 
-apr_status_t h2_headers_add_h1(apr_table_t *headers, apr_pool_t *pool, 
+apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool, 
                                const char *name, size_t nlen,
                                const char *value, size_t vlen)
 {

modules/http2/h2_util.h

@@ -341,19 +341,21 @@ const char *h2_util_base64url_encode(const char *data,
 
 int h2_util_ignore_header(const char *name);
 
+struct h2_headers;
+
 typedef struct h2_ngheader {
     nghttp2_nv *nv;
     apr_size_t nvlen;
 } h2_ngheader;
 
-h2_ngheader *h2_util_ngheader_make(apr_pool_t *p, apr_table_t *header);
+apr_status_t h2_res_create_ngtrailer(h2_ngheader **ph, apr_pool_t *p, 
-h2_ngheader *h2_util_ngheader_make_res(apr_pool_t *p, 
+                                     struct h2_headers *headers); 
-                                       int http_status, 
+apr_status_t h2_res_create_ngheader(h2_ngheader **ph, apr_pool_t *p, 
-                                       apr_table_t *header);
+                                    struct h2_headers *headers); 
-h2_ngheader *h2_util_ngheader_make_req(apr_pool_t *p, 
+apr_status_t h2_req_create_ngheader(h2_ngheader **ph, apr_pool_t *p, 
-                                       const struct h2_request *req);
+                                    const struct h2_request *req);
 
-apr_status_t h2_headers_add_h1(apr_table_t *headers, apr_pool_t *pool, 
+apr_status_t h2_req_add_header(apr_table_t *headers, apr_pool_t *pool, 
                                const char *name, size_t nlen,
                                const char *value, size_t vlen);