httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: svn commit: r1715363 - in /httpd/httpd/trunk/modules/http2: h2_request.c h2_response.h h2_session.c h2_stream.c h2_stream.h h2_util.c h2_util.h
Date Fri, 20 Nov 2015 16:27:18 GMT
Hmm, "header field names" usually refers to header names.

On Fri, Nov 20, 2015 at 5:12 PM, Stefan Eissing
<stefan.eissing@greenbytes.de> wrote:
> Yes, but I interpret that as the header fields, not any tokens that refer to header names.
>
>> Am 20.11.2015 um 17:04 schrieb Jim Jagielski <jim@jaguNET.com>:
>>
>> Ugg... I *just* noticed:
>>
>>    However, header field names MUST be converted to lowercase
>>    prior to their encoding in HTTP/2. A request or response
>>    containing uppercase header field names MUST be treated as
>>    malformed
>>
>>
>>
>>> On Nov 20, 2015, at 10:44 AM, Stefan Eissing <stefan.eissing@greenbytes.de>
wrote:
>>>
>>> Most of this is discussed here: https://httpwg.github.io/specs/rfc7540.html#HttpHeaders
>>>
>>> Basically HTTP/2 defines its own connection properties, so several things which
are announced/controlled
>>> by Connection: and other headers do not apply to HTTP/2 connections. So, the
"Connection:" header itself
>>> was obsoleted.
>>>
>>> mod_http2 has to translate between both worlds a bit and is doing so probably
incompletely so now, so
>>> the discussion about this is very useful.
>>>
>>> There are two conversions for mod_http2 to make:
>>> 1. request headers (H2 -> H1):
>>> 2. response headers (H1 -> H2):
>>>
>>> For 1. certain headers should not arrive at all. If they do, we can either ignore
or generate an error and deny the request. For example "Connection: ..." should never arrive.
Same for "Transfer-Encoding: ". The current implementation ignores. One could argue for deny.
>>> For 2. certain headers we cannot send out and we need to take proper actions
for that. For example a "TE: deflate" we cannot process like this. Here more checks need to
be added.
>>>
>>> //Stefan
>>>
>>>> Am 20.11.2015 um 16:25 schrieb Yann Ylavic <ylavic.dev@gmail.com>:
>>>>
>>>> On Fri, Nov 20, 2015 at 2:58 PM,  <icing@apache.org> wrote:
>>>>> Author: icing
>>>>> Date: Fri Nov 20 13:58:32 2015
>>>>> New Revision: 1715363
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1715363&view=rev
>>>>> Log:
>>>>> incoming trailers passed into chunked request bodies, outgoing trailers
not supported yet
>>>>>
>>>>> Modified:
>>>> []
>>>>>  httpd/httpd/trunk/modules/http2/h2_util.c
>>>> []
>>>>>
>>>>>
>>>>> Modified: httpd/httpd/trunk/modules/http2/h2_util.c
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_util.c?rev=1715363&r1=1715362&r2=1715363&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/modules/http2/h2_util.c (original)
>>>>> +++ httpd/httpd/trunk/modules/http2/h2_util.c Fri Nov 20 13:58:32 2015
>>>> []
>>>>> @@ -879,3 +894,94 @@ h2_ngheader *h2_util_ngheader_make_req(a
>>>>>   return ngh;
>>>>> }
>>>>>
>>>>> +/*******************************************************************************
>>>>> + * header HTTP/1 <-> HTTP/2 conversions
>>>>> + ******************************************************************************/
>>>>> +
>>>>> +
>>>>> +typedef struct {
>>>>> +    const char *name;
>>>>> +    size_t len;
>>>>> +} literal;
>>>>> +
>>>>> +#define H2_DEF_LITERAL(n)   { (n), (sizeof(n)-1) }
>>>>> +#define H2_ALEN(a)          (sizeof(a)/sizeof((a)[0]))
>>>>> +#define H2_LIT_ARGS(a)      (a),H2_ALEN(a)
>>>>> +
>>>>> +static literal IgnoredRequestHeaders[] = {
>>>>> +    H2_DEF_LITERAL("host"),
>>>>> +    H2_DEF_LITERAL("expect"),
>>>>> +    H2_DEF_LITERAL("upgrade"),
>>>>> +    H2_DEF_LITERAL("connection"),
>>>>> +    H2_DEF_LITERAL("keep-alive"),
>>>>> +    H2_DEF_LITERAL("http2-settings"),
>>>>> +    H2_DEF_LITERAL("proxy-connection"),
>>>>> +    H2_DEF_LITERAL("transfer-encoding"),
>>>>> +};
>>>>
>>>> Shouldn't we also include all the tokens from the Connection header?
>>>> (obviously not feasible with this only blacklist)
>>>>
>>>>
>>>>> +static literal IgnoredRequestTrailers[] = { /* Ignore, see rfc7230,
ch. 4.1.2 */
>>>>> +    H2_DEF_LITERAL("te"),
>>>>> +    H2_DEF_LITERAL("host"),
>>>>> +    H2_DEF_LITERAL("range"),
>>>>> +    H2_DEF_LITERAL("cookie"),
>>>>> +    H2_DEF_LITERAL("expect"),
>>>>> +    H2_DEF_LITERAL("pragma"),
>>>>> +    H2_DEF_LITERAL("max-forwards"),
>>>>> +    H2_DEF_LITERAL("cache-control"),
>>>>> +    H2_DEF_LITERAL("authorization"),
>>>>> +    H2_DEF_LITERAL("content-length"),
>>>>> +    H2_DEF_LITERAL("proxy-authorization"),
>>>>> +};
>>>>
>>>> No notion of announced trailers in HTTP2, with eg. the request header
>>>> "Trailer: token1, token2, ..." (any non-announced trailer would be
>>>> ignored), something we could (always/optionally) enforce?
>>>>
>>>>> +static literal IgnoredResponseTrailers[] = {
>>>>> +    H2_DEF_LITERAL("age"),
>>>>> +    H2_DEF_LITERAL("date"),
>>>>> +    H2_DEF_LITERAL("vary"),
>>>>> +    H2_DEF_LITERAL("cookie"),
>>>>> +    H2_DEF_LITERAL("expires"),
>>>>> +    H2_DEF_LITERAL("warning"),
>>>>> +    H2_DEF_LITERAL("location"),
>>>>> +    H2_DEF_LITERAL("retry-after"),
>>>>> +    H2_DEF_LITERAL("cache-control"),
>>>>> +    H2_DEF_LITERAL("www-authenticate"),
>>>>> +    H2_DEF_LITERAL("proxy-authenticate"),
>>>>> +};
>>>>
>>>> Likewise, shouldn't we check whether the request associated to the
>>>> response contains a "TE: trailers" or otherwise use no trailer?
>>>>
>>>>
>>>> I'm a bit confused about what is included-in/overwritten-by the HTTP2
>>>> framing and what remains from HTTP1 RFCs/requirements.
>>>>
>>>>
>>>> Regards,
>>>> Yann.
>>

Mime
View raw message