You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Eissing <st...@greenbytes.de> on 2015/08/24 18:29:47 UTC

protocols and mod_h2 consolidation

As you might have noticed, I am doing some cleanup work on mod_h2 and want to create THE PATCH for backporting soon. The new core directives "Protocols" and "ProtocolsHonorOrder" are implemented, such as on a server or virtual host one may write

   Protocols  h2c http/1.1      # enable h2c and http/1.1 here, preference decided by client ordering

   Protocols  http/1.1          # only allow http/1.1 here

   Protocols  h2 http/1.1       # enable h2c and http/1.1 here, with preference to h2
   ProtocolsHonorOrder on

   Protocols  YOLO h2 http/1.1  # enable YOLO protocol among others, which has no effect unless a module claims to implement it. 

With no "Protocols" specified, all protocols offered by the client and implemented by a module may be negotiated with the preference being in the client's ordering. This allows "drop in" of protocol modules without further configuration.

Inheritance of "Protocols" between servers and virtual hosts is copyOrReplace, meaning vhosts without Protocols inherit from base server. vhosts with a "Protocols" ignore base server lists. This allows restrictions in the vhosts at the cost of some redundancy. But Protocols differences between base servers and vhosts seem to be rather esotheric right now. So, I tried to keep it simple.

The "ProtocolsHonorOrder" name I tried to name in the spirit of the SSLHonorCipherOrder directive. No strong feelings about it.

I hope this works for everyone. The next weeks might be a good time to think about it and propose any changes and correct my mistakes.

Thanks.

<green/>bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782




Re: protocols and mod_h2 consolidation

Posted by Eric Covener <co...@gmail.com>.
On Mon, Aug 24, 2015 at 12:29 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
> I hope this works for everyone. The next weeks might be a good time to think about it and propose any changes and correct my mistakes.

Look good in concept to me

Re: protocols and mod_h2 consolidation

Posted by Gregg Smith <gl...@gknw.net>.
Stefan,

Changes worked great. Thanks.

Gregg

On 8/25/2015 3:43 AM, Stefan Eissing wrote:
> Gregg,
>
> I just checked in changes that should fix the warnings you mentioned. Thanks for reviewing this. As to the SERVER_PROTOCOL, I have no idea yet what may cause this.
>
> //Stefan
>
>> Am 25.08.2015 um 06:11 schrieb Gregg Smith<gl...@gknw.net>:
>>
>> Hi,
>>
>> On 8/24/2015 9:29 AM, Stefan Eissing wrote:
>>
>>> I hope this works for everyone. The next weeks might be a good time to think about it and propose any changes and correct my mistakes.
>> There are two things that go bump on my lowest non-eol version of MSVC.
>> h2_worker.c
>> .\h2_worker.c(113) : error C2440: 'function' : cannot convert from 'void *(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
>> .\h2_worker.c(113) : warning C4022: 'apr_thread_create' : pointer mismatch for actual parameter 3
>>
>> Casting execute to apr_thread_start_t (what apr_thread_create expects) seems to quiet the compiler and FireFox seems to work with it.
>>
>> h2_session.c
>> .\h2_session.c(1079) : error C2440: 'type cast' : cannot convert from 'int' to 'nghttp2_data_source'
>> .\h2_session.c(1081) : warning C4047: 'initializing' : 'int' differs in levels of indirection from 'nghttp2_data_source_read_callback'
>>
>> This cast to a union type, this MSVC version's docs on Unions has a note in the comments mentioning;
>>> We recommend that you do not use a union to cast data from one data type to another because union members occupy the same address in memory.
>>>
>>> There is no data-conversion support for unions. The results of interchanging writes and reads between union members of different types are unpredictable and depend on a variety of reasons.
>>>
>> They are evidently enforcing this. I just removed the cast and it builds and Firefox seems to work.
>>
>> Other nits of interest or not;
>> h2_stream_set.c
>> .\h2_stream_set.c(139) : warning C4028: formal parameter 2 different from declaration
>>
>> The function is prototyped as: h2_stream_set_match_fn *match
>> but in h2_stream_set.c it is: h2_stream_set_match_fn match
>>
>> I get 2 different results for the $ENV{'SERVER_PROTOCOL'} in my perl script depending on the machine. My server gives me "HTTP/1.1" and running locally on my build machine I get "INCLUDED." Firefox says it is HTTP/2.0 200 OK so both results in this $ENV seem incorrect. I'm not sure why the difference as mod_h2 is configured same on both.
>>
>> Regards,
>> Gregg
>>
>>
>>
>>
>>
> <green/>bytes GmbH
> Hafenweg 16, 48155 Münster, Germany
> Phone: +49 251 2807760. Amtsgericht Münster: HRB5782
>
>
>


Re: protocols and mod_h2 consolidation

Posted by Stefan Eissing <st...@greenbytes.de>.
Gregg,

I just checked in changes that should fix the warnings you mentioned. Thanks for reviewing this. As to the SERVER_PROTOCOL, I have no idea yet what may cause this.

//Stefan

> Am 25.08.2015 um 06:11 schrieb Gregg Smith <gl...@gknw.net>:
> 
> Hi,
> 
> On 8/24/2015 9:29 AM, Stefan Eissing wrote:
> 
>> I hope this works for everyone. The next weeks might be a good time to think about it and propose any changes and correct my mistakes.
> 
> There are two things that go bump on my lowest non-eol version of MSVC.
> h2_worker.c
> .\h2_worker.c(113) : error C2440: 'function' : cannot convert from 'void *(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
> .\h2_worker.c(113) : warning C4022: 'apr_thread_create' : pointer mismatch for actual parameter 3
> 
> Casting execute to apr_thread_start_t (what apr_thread_create expects) seems to quiet the compiler and FireFox seems to work with it.
> 
> h2_session.c
> .\h2_session.c(1079) : error C2440: 'type cast' : cannot convert from 'int' to 'nghttp2_data_source'
> .\h2_session.c(1081) : warning C4047: 'initializing' : 'int' differs in levels of indirection from 'nghttp2_data_source_read_callback'
> 
> This cast to a union type, this MSVC version's docs on Unions has a note in the comments mentioning;
>> 
>> We recommend that you do not use a union to cast data from one data type to another because union members occupy the same address in memory.
>> 
>> There is no data-conversion support for unions. The results of interchanging writes and reads between union members of different types are unpredictable and depend on a variety of reasons.
>> 
> 
> They are evidently enforcing this. I just removed the cast and it builds and Firefox seems to work.
> 
> Other nits of interest or not;
> h2_stream_set.c
> .\h2_stream_set.c(139) : warning C4028: formal parameter 2 different from declaration
> 
> The function is prototyped as: h2_stream_set_match_fn *match
> but in h2_stream_set.c it is: h2_stream_set_match_fn match
> 
> I get 2 different results for the $ENV{'SERVER_PROTOCOL'} in my perl script depending on the machine. My server gives me "HTTP/1.1" and running locally on my build machine I get "INCLUDED." Firefox says it is HTTP/2.0 200 OK so both results in this $ENV seem incorrect. I'm not sure why the difference as mod_h2 is configured same on both.
> 
> Regards,
> Gregg
> 
> 
> 
> 
> 

<green/>bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782




Re: protocols and mod_h2 consolidation

Posted by Stefan Eissing <st...@greenbytes.de>.
Ok, I see that warning flags vary on platforms. That's natural. However if our most common (I assume) dev platform does not enable the superset of all, then we get errors/warnings continuously as people will check in code that fails elsewhere. And I do not think that adding things like

#ifndef _MSC_VER
#pragma GCC diagnostic ignored "-Wmissing-braces"
#endif

are helping anyone. 

What can we do that this becomes unnecessary? I am not a MSVC user, but either we need to disable "missing-braces" in that project file or we need to add it to the gcc CFLAGS in maintainer-mode. Or?

//Stefan


> Am 25.08.2015 um 06:11 schrieb Gregg Smith <gl...@gknw.net>:
> 
> Hi,
> 
> On 8/24/2015 9:29 AM, Stefan Eissing wrote:
> 
>> I hope this works for everyone. The next weeks might be a good time to think about it and propose any changes and correct my mistakes.
> 
> There are two things that go bump on my lowest non-eol version of MSVC.
> h2_worker.c
> .\h2_worker.c(113) : error C2440: 'function' : cannot convert from 'void *(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
> .\h2_worker.c(113) : warning C4022: 'apr_thread_create' : pointer mismatch for actual parameter 3
> 
> Casting execute to apr_thread_start_t (what apr_thread_create expects) seems to quiet the compiler and FireFox seems to work with it.
> 
> h2_session.c
> .\h2_session.c(1079) : error C2440: 'type cast' : cannot convert from 'int' to 'nghttp2_data_source'
> .\h2_session.c(1081) : warning C4047: 'initializing' : 'int' differs in levels of indirection from 'nghttp2_data_source_read_callback'
> 
> This cast to a union type, this MSVC version's docs on Unions has a note in the comments mentioning;
>> 
>> We recommend that you do not use a union to cast data from one data type to another because union members occupy the same address in memory.
>> 
>> There is no data-conversion support for unions. The results of interchanging writes and reads between union members of different types are unpredictable and depend on a variety of reasons.
>> 
> 
> They are evidently enforcing this. I just removed the cast and it builds and Firefox seems to work.
> 
> Other nits of interest or not;
> h2_stream_set.c
> .\h2_stream_set.c(139) : warning C4028: formal parameter 2 different from declaration
> 
> The function is prototyped as: h2_stream_set_match_fn *match
> but in h2_stream_set.c it is: h2_stream_set_match_fn match
> 
> I get 2 different results for the $ENV{'SERVER_PROTOCOL'} in my perl script depending on the machine. My server gives me "HTTP/1.1" and running locally on my build machine I get "INCLUDED." Firefox says it is HTTP/2.0 200 OK so both results in this $ENV seem incorrect. I'm not sure why the difference as mod_h2 is configured same on both.
> 
> Regards,
> Gregg
> 
> 
> 
> 
> 

<green/>bytes GmbH
Hafenweg 16, 48155 Münster, Germany
Phone: +49 251 2807760. Amtsgericht Münster: HRB5782




Re: protocols and mod_h2 consolidation

Posted by Gregg Smith <gl...@gknw.net>.
On 8/25/2015 5:57 PM, William A Rowe Jr wrote:
> On Aug 24, 2015 11:43 PM, "Gregg Smith"<gl...@gknw.net>  wrote:
>> On 8/24/2015 9:29 AM, Stefan Eissing wrote:
>>
>>> I hope this works for everyone. The next weeks might be a good time to
> think about it and propose any changes and correct my mistakes.
>> There are two things that go bump on my lowest non-eol version of MSVC.
>> h2_worker.c
>> .\h2_worker.c(113) : error C2440: 'function' : cannot convert from 'void
> *(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
>> .\h2_worker.c(113) : warning C4022: 'apr_thread_create' : pointer
> mismatch for actual parameter 3
>> Casting execute to apr_thread_start_t (what apr_thread_create expects)
> seems to quiet the compiler and FireFox seems to work with it.
>
> That sounds most unsafe.  Is this a mismatch between DECLARE and
> DECLARE_NONSTD?
>
> Working right now on Linux but will take a few min to look at both issues
> you flagged.
I'm sure it was, but I wanted it to build. I did not commit that.
Stefen has dealt with these issues properly I'd imagine in r1697634


Re: protocols and mod_h2 consolidation

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Aug 24, 2015 11:43 PM, "Gregg Smith" <gl...@gknw.net> wrote:
>
> On 8/24/2015 9:29 AM, Stefan Eissing wrote:
>
>> I hope this works for everyone. The next weeks might be a good time to
think about it and propose any changes and correct my mistakes.
>
> There are two things that go bump on my lowest non-eol version of MSVC.
> h2_worker.c
> .\h2_worker.c(113) : error C2440: 'function' : cannot convert from 'void
*(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
> .\h2_worker.c(113) : warning C4022: 'apr_thread_create' : pointer
mismatch for actual parameter 3
>
> Casting execute to apr_thread_start_t (what apr_thread_create expects)
seems to quiet the compiler and FireFox seems to work with it.

That sounds most unsafe.  Is this a mismatch between DECLARE and
DECLARE_NONSTD?

Working right now on Linux but will take a few min to look at both issues
you flagged.

Re: protocols and mod_h2 consolidation

Posted by Gregg Smith <gl...@gknw.net>.
Hi,

On 8/24/2015 9:29 AM, Stefan Eissing wrote:

> I hope this works for everyone. The next weeks might be a good time to think about it and propose any changes and correct my mistakes.

There are two things that go bump on my lowest non-eol version of MSVC.
h2_worker.c
.\h2_worker.c(113) : error C2440: 'function' : cannot convert from 'void 
*(__cdecl *)(apr_thread_t *,void *)' to 'apr_thread_start_t'
.\h2_worker.c(113) : warning C4022: 'apr_thread_create' : pointer 
mismatch for actual parameter 3

Casting execute to apr_thread_start_t (what apr_thread_create expects) 
seems to quiet the compiler and FireFox seems to work with it.

h2_session.c
.\h2_session.c(1079) : error C2440: 'type cast' : cannot convert from 
'int' to 'nghttp2_data_source'
.\h2_session.c(1081) : warning C4047: 'initializing' : 'int' differs in 
levels of indirection from 'nghttp2_data_source_read_callback'

This cast to a union type, this MSVC version's docs on Unions has a note 
in the comments mentioning;
>
> We recommend that you do not use a union to cast data from one data 
> type to another because union members occupy the same address in memory.
>
> There is no data-conversion support for unions. The results of 
> interchanging writes and reads between union members of different 
> types are unpredictable and depend on a variety of reasons.
>

They are evidently enforcing this. I just removed the cast and it builds 
and Firefox seems to work.

Other nits of interest or not;
h2_stream_set.c
.\h2_stream_set.c(139) : warning C4028: formal parameter 2 different 
from declaration

The function is prototyped as: h2_stream_set_match_fn *match
but in h2_stream_set.c it is: h2_stream_set_match_fn match

I get 2 different results for the $ENV{'SERVER_PROTOCOL'} in my perl 
script depending on the machine. My server gives me "HTTP/1.1" and 
running locally on my build machine I get "INCLUDED." Firefox says it is 
HTTP/2.0 200 OK so both results in this $ENV seem incorrect. I'm not 
sure why the difference as mod_h2 is configured same on both.

Regards,
Gregg