RE: svn commit: r1714219 - in /httpd/httpd/trunk: docs/manual/mod/ modules/http2/

[Bert Huijben <be...@qqmail.nl>]

> -----Original Message-----
> From: icing@apache.org [mailto:icing@apache.org]
> Sent: vrijdag 13 november 2015 15:54
> To: cvs@httpd.apache.org
> Subject: svn commit: r1714219 - in /httpd/httpd/trunk: docs/manual/mod/
> modules/http2/
> 
> Author: icing
> Date: Fri Nov 13 14:54:15 2015
> New Revision: 1714219
> 
> URL: http://svn.apache.org/viewvc?rev=1714219&view=rev
> Log:
> new directive H2Push on/off to en-/disable HTTP/2 server pushes. Server
> pushes are recognized by Link: headers in responses that carry the
> rel=preload parameter
> 


> Modified: httpd/httpd/trunk/modules/http2/h2_request.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_reque
> st.c?rev=1714219&r1=1714218&r2=1714219&view=diff
> ==========================================================
> ====================
> --- httpd/httpd/trunk/modules/http2/h2_request.c (original)
> +++ httpd/httpd/trunk/modules/http2/h2_request.c Fri Nov 13 14:54:15
> 2015
> @@ -31,11 +31,22 @@
> 
>  h2_request *h2_request_create(int id, apr_pool_t *pool)
>  {
> +    return h2_request_createn(id, pool, NULL, NULL, NULL, NULL, NULL);
> +}
> +
> +h2_request *h2_request_createn(int id, apr_pool_t *pool,
> +                               const char *method, const char *scheme,
> +                               const char *authority, const char *path,
> +                               apr_table_t *header)
> +{
>      h2_request *req = apr_pcalloc(pool, sizeof(h2_request));
> 
> -    req->id = id;
> -    req->headers = apr_table_make(pool, 10);
> -    req->content_length = -1;
> +    req->id             = id;
> +    req->method         = method;
> +    req->scheme         = scheme;
> +    req->authority      = authority;
> +    req->path           = path;
> +    req->headers        = header? header : apr_table_make(pool, 10);
> 
>      return req;
>  }
> @@ -44,45 +55,26 @@ void h2_request_destroy(h2_request *req)
>  {
>  }
> 
> +static apr_status_t inspect_clen(h2_request *req, const char *s)
> +{
> +    char *end;
> +    req->content_length = apr_strtoi64(s, &end, 10);
> +    return (s == end)? APR_EINVAL : APR_SUCCESS;
> +}
> +
>  static apr_status_t add_h1_header(h2_request *req, apr_pool_t *pool,
>                                    const char *name, size_t nlen,
>                                    const char *value, size_t vlen)
>  {
>      char *hname, *hvalue;
> 
> -    if (H2_HD_MATCH_LIT("transfer-encoding", name, nlen)) {
> -        if (!apr_strnatcasecmp("chunked", value)) {
> -            /* This should never arrive here in a HTTP/2 request */
> -            ap_log_perror(APLOG_MARK, APLOG_ERR, APR_BADARG, pool,
> -                          APLOGNO(02945)
> -                          "h2_request: 'transfer-encoding: chunked' received");
> -            return APR_BADARG;
> -        }
> -    }
> -    else if (H2_HD_MATCH_LIT("content-length", name, nlen)) {
> -        char *end;
> -        req->content_length = apr_strtoi64(value, &end, 10);
> -        if (value == end) {
> -            ap_log_perror(APLOG_MARK, APLOG_WARNING, APR_EINVAL, pool,
> -                          APLOGNO(02959)
> -                          "h2_request(%d): content-length value not parsed: %s",
> -                          req->id, value);
> -            return APR_EINVAL;
> -        }
> -        req->chunked = 0;
> -    }
> -    else if (H2_HD_MATCH_LIT("content-type", name, nlen)) {
> -        /* If we see a content-type and have no length (yet),
> -         * we need to chunk. */
> -        req->chunked = (req->content_length == -1);
> -    }
> -    else if ((req->seen_host && H2_HD_MATCH_LIT("host", name, nlen))
> -             || H2_HD_MATCH_LIT("expect", name, nlen)
> -             || H2_HD_MATCH_LIT("upgrade", name, nlen)
> -             || H2_HD_MATCH_LIT("connection", name, nlen)
> -             || H2_HD_MATCH_LIT("proxy-connection", name, nlen)
> -             || H2_HD_MATCH_LIT("keep-alive", name, nlen)
> -             || H2_HD_MATCH_LIT("http2-settings", name, nlen)) {
> +    if (H2_HD_MATCH_LIT("expect", name, nlen)
> +        || H2_HD_MATCH_LIT("upgrade", name, nlen)
> +        || H2_HD_MATCH_LIT("connection", name, nlen)
> +        || H2_HD_MATCH_LIT("proxy-connection", name, nlen)
> +        || H2_HD_MATCH_LIT("transfer-encoding", name, nlen)
> +        || H2_HD_MATCH_LIT("keep-alive", name, nlen)
> +        || H2_HD_MATCH_LIT("http2-settings", name, nlen)) {
>          /* ignore these. */
>          return APR_SUCCESS;


Not added in this revision, but this is not 100% to the spec:

http2-settings is only special if it is referenced in the upgrade header, so dropping "connection" would be enough.

Expect is still 100% valid in http/2; as are trailing headers, which used transfer-encoding in http/1. So that header shouldn't be dropped.

Not sure about keep-alive. But if I remember correctly the spec says that it needs to be referenced from connection too, to be applied at the connection layer. (But I wouldn't be surprised if actual implementation think different there)

	Bert