Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Stefan Eissing]

Yeah, I tried a 0 len read. Unfortunately, the core input filter is not happy 
about that:

core_filters.c, line 231
AP_DEBUG_ASSERT(readbytes > 0);

> Am 12.10.2015 um 10:25 schrieb Plüm, Rüdiger, Vodafone Group 
> :
> 
> How about the following patch?
> 
> This separates the triggering of the SSL Handshake and the possible reading 
> of the 24 bytes:
> 
> Index: h2_h2.c
> ===   
> --- h2_h2.c (revision 1707437)
> +++ h2_h2.c (working copy)
> @@ -154,7 +154,7 @@
> 
> temp = apr_brigade_create(c->pool, c->bucket_alloc);
> status = ap_get_brigade(c->input_filters, temp,
> -AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24);
> +AP_MODE_INIT, APR_BLOCK_READ, 0);
> 
> if (status == APR_SUCCESS) {
> if (h2_ctx_protocol_get(c)
> @@ -171,23 +171,31 @@
> char *s = NULL;
> apr_size_t slen;
> 
> -apr_brigade_pflatten(temp, , , c->pool);
> -if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
> -ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
> -  "h2_h2, direct mode detected");
> -h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
> +status = ap_get_brigade(c->input_filters, temp,
> +AP_MODE_SPECULATIVE, 
> APR_BLOCK_READ, 24);
> +if (status == APR_SUCCESS) {
> +apr_brigade_pflatten(temp, , , c->pool);
> +if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
> +ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
> +  "h2_h2, direct mode detected");
> +h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
> +}
> +else {
> +ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
> +  "h2_h2, not detected in %d bytes: 
> %s",
> +  (int)slen, s);
> +}
> }
> -else {
> -ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
> -  "h2_h2, not detected in %d bytes: %s",
> -  (int)slen, s);
> -}
> }
> +else {
> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
> +  "h2_h2, error reading 24 bytes 
> speculative");
> +}
> }
> }
> else {
> ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
> -  "h2_h2, error reading 24 bytes speculative");
> +  "h2_h2, Failed to init connection");
> }
> apr_brigade_destroy(temp);
> }
> 
> This would still block in the non ssl case if directmode is not set to off 
> explicitly. I would propose to change the default behaviour of directmode 
> here to off as directmode seems to be something very special to me that 
> should be explicitly enabled.
> 
> Regards
> 
> Rüdiger
> 
>> -Original Message-
>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>> Sent: Sonntag, 11. Oktober 2015 19:54
>> To: dev@httpd.apache.org
>> Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
>> loaded
>> 
>> Ok, in ssl_engine_io.c, lines 1426+ I see a hint:
>> 
>>/* XXX: we could actually move ssl_io_filter_handshake to an
>> * ap_hook_process_connection but would still need to call it for
>> * AP_MODE_INIT for protocols that may upgrade the connection
>> * rather than have SSLEngine On configured.
>> */
>>if ((status = ssl_io_filter_handshake(inctx->filter_ctx)) !=
>> APR_SUCCESS) {
>>return ssl_io_filter_error(f, bb, status);
>>}
>> 
>> The handshake handling gets triggered on a filter, so it all happens on
>> READs (or WRITEs), which explains why connection hooks do not see it. This
>> is quite some code which looks like we do not want to touch for a "quick
>> fix". Also, there is no quick fix inside mod_http2 as before handshake,
>> the SNI might also not be there yet, thus the vhost cannot be determined.
>> etc.
>> 
>> For the current release, I think we should leave it as it is and advise
>> that current mod_http2 is incompatible with things like nntp.
>> 
>> For the next release, I'd like the server to perform the handshake at a
>> defined time, so other modules can rely on protocols being selected 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Stefan Eissing]

I am about to change that.

> Am 12.10.2015 um 12:48 schrieb Plüm, Rüdiger, Vodafone Group 
> :
> 
> 
> 
>> -Original Message-
>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>> Sent: Montag, 12. Oktober 2015 10:58
>> To: dev@httpd.apache.org
>> Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
>> loaded
>> 
>> The AP_MODE_INIT triggers the handshake nicely. But the protocol switching
>> still happens
>> on the first read. currently looking how to trigger that best.
> 
> Even in the ALPN case the protocol switching happens on the first read after 
> the handshake?
> 
> Regards
> 
> Rüdiger
> 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Yann Ylavic]

The key here is using AP_MODE_INIT first (where 0 make sense) before
AP_MODE_SPECULATIVE.


On Mon, Oct 12, 2015 at 10:36 AM, Stefan Eissing
 wrote:
> Yeah, I tried a 0 len read. Unfortunately, the core input filter is not happy 
> about that:
>
> core_filters.c, line 231
> AP_DEBUG_ASSERT(readbytes > 0);
>
>> Am 12.10.2015 um 10:25 schrieb Plüm, Rüdiger, Vodafone Group 
>> :
>>
>> How about the following patch?
>>
>> This separates the triggering of the SSL Handshake and the possible reading 
>> of the 24 bytes:
>>
>> Index: h2_h2.c
>> ===
>> --- h2_h2.c (revision 1707437)
>> +++ h2_h2.c (working copy)
>> @@ -154,7 +154,7 @@
>>
>> temp = apr_brigade_create(c->pool, c->bucket_alloc);
>> status = ap_get_brigade(c->input_filters, temp,
>> -AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24);
>> +AP_MODE_INIT, APR_BLOCK_READ, 0);
>>
>> if (status == APR_SUCCESS) {
>> if (h2_ctx_protocol_get(c)
>> @@ -171,23 +171,31 @@
>> char *s = NULL;
>> apr_size_t slen;
>>
>> -apr_brigade_pflatten(temp, , , c->pool);
>> -if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
>> -ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
>> -  "h2_h2, direct mode detected");
>> -h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
>> +status = ap_get_brigade(c->input_filters, temp,
>> +AP_MODE_SPECULATIVE, 
>> APR_BLOCK_READ, 24);
>> +if (status == APR_SUCCESS) {
>> +apr_brigade_pflatten(temp, , , c->pool);
>> +if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) 
>> {
>> +ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
>> +  "h2_h2, direct mode detected");
>> +h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
>> +}
>> +else {
>> +ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>> +  "h2_h2, not detected in %d bytes: 
>> %s",
>> +  (int)slen, s);
>> +}
>> }
>> -else {
>> -ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>> -  "h2_h2, not detected in %d bytes: %s",
>> -  (int)slen, s);
>> -}
>> }
>> +else {
>> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
>> +  "h2_h2, error reading 24 bytes 
>> speculative");
>> +}
>> }
>> }
>> else {
>> ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
>> -  "h2_h2, error reading 24 bytes speculative");
>> +  "h2_h2, Failed to init connection");
>> }
>> apr_brigade_destroy(temp);
>> }
>>
>> This would still block in the non ssl case if directmode is not set to off 
>> explicitly. I would propose to change the default behaviour of directmode 
>> here to off as directmode seems to be something very special to me that 
>> should be explicitly enabled.
>>
>> Regards
>>
>> Rüdiger
>>
>>> -Original Message-
>>> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
>>> Sent: Sonntag, 11. Oktober 2015 19:54
>>> To: dev@httpd.apache.org
>>> Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
>>> loaded
>>>
>>> Ok, in ssl_engine_io.c, lines 1426+ I see a hint:
>>>
>>>/* XXX: we could actually move ssl_io_filter_handshake to an
>>> * ap_hook_process_connection but would still need to call it for
>>> * AP_MODE_INIT for protocols that may upgrade the connection
>>> * rather than have SSLEngine On configured.
>>> */
>>>if ((status = ssl_io_filter_handshake(inctx->filter_ctx)) !=
>>> APR_SUCCESS) {
>>>return ssl_io_filter_error(f, bb, status);
>>>}
>>>
>>> The handshake handling gets triggered on a filter, so it all happens on
>>> READs (or WRITEs), which explains why connection hooks do not see it. This
>>> is quite some code which looks like we do not want to touch for a "quick
>>> fix". Also, there is no quick fix inside mod_http2 as before handshake,
>>> the SNI might also not be there yet, thus the vhost cannot be determined.
>>> etc.
>>>
>>> For the current release, I think we should leave it as it is and advise
>>> that current mod_http2 is incompatible with things like nntp.
>>>
>>> For the next release, I'd 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Yann Ylavic]

Also, I wonder if we don't need:

Index: modules/http2/h2_task_input.c
===
--- modules/http2/h2_task_input.c(revision 1707888)
+++ modules/http2/h2_task_input.c(working copy)
@@ -116,7 +116,7 @@ apr_status_t h2_task_input_read(h2_task_input *inp
 }

 if (mode == AP_MODE_INIT) {
-return APR_SUCCESS;
+return ap_get_brigade(f->c->input_filters, bb, mode, block, readbytes);
 }

 if (input->bb) {
--

We shouldn't since h2_task_input_read() is called at NETWORK level
(hence after mod_ssl which "eats" AP_MODE_INIT), but it looks more
correct (as it would be in mod_ssl) since anyway the
core_input_filter() can handle it.


On Mon, Oct 12, 2015 at 10:42 AM, Yann Ylavic  wrote:
> The key here is using AP_MODE_INIT first (where 0 make sense) before
> AP_MODE_SPECULATIVE.
>
>
> On Mon, Oct 12, 2015 at 10:36 AM, Stefan Eissing
>  wrote:
>> Yeah, I tried a 0 len read. Unfortunately, the core input filter is not 
>> happy about that:
>>
>> core_filters.c, line 231
>> AP_DEBUG_ASSERT(readbytes > 0);
>>
>>> Am 12.10.2015 um 10:25 schrieb Plüm, Rüdiger, Vodafone Group 
>>> :
>>>
>>> How about the following patch?
>>>
>>> This separates the triggering of the SSL Handshake and the possible reading 
>>> of the 24 bytes:
>>>
>>> Index: h2_h2.c
>>> ===
>>> --- h2_h2.c (revision 1707437)
>>> +++ h2_h2.c (working copy)
>>> @@ -154,7 +154,7 @@
>>>
>>> temp = apr_brigade_create(c->pool, c->bucket_alloc);
>>> status = ap_get_brigade(c->input_filters, temp,
>>> -AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24);
>>> +AP_MODE_INIT, APR_BLOCK_READ, 0);
>>>
>>> if (status == APR_SUCCESS) {
>>> if (h2_ctx_protocol_get(c)
>>> @@ -171,23 +171,31 @@
>>> char *s = NULL;
>>> apr_size_t slen;
>>>
>>> -apr_brigade_pflatten(temp, , , c->pool);
>>> -if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
>>> -ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
>>> -  "h2_h2, direct mode detected");
>>> -h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
>>> +status = ap_get_brigade(c->input_filters, temp,
>>> +AP_MODE_SPECULATIVE, 
>>> APR_BLOCK_READ, 24);
>>> +if (status == APR_SUCCESS) {
>>> +apr_brigade_pflatten(temp, , , c->pool);
>>> +if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 
>>> 24)) {
>>> +ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
>>> +  "h2_h2, direct mode detected");
>>> +h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
>>> +}
>>> +else {
>>> +ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>>> +  "h2_h2, not detected in %d 
>>> bytes: %s",
>>> +  (int)slen, s);
>>> +}
>>> }
>>> -else {
>>> -ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
>>> -  "h2_h2, not detected in %d bytes: 
>>> %s",
>>> -  (int)slen, s);
>>> -}
>>> }
>>> +else {
>>> +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
>>> +  "h2_h2, error reading 24 bytes 
>>> speculative");
>>> +}
>>> }
>>> }
>>> else {
>>> ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
>>> -  "h2_h2, error reading 24 bytes speculative");
>>> +  "h2_h2, Failed to init connection");
>>> }
>>> apr_brigade_destroy(temp);
>>> }
>>>
>>> This would still block in the non ssl case if directmode is not set to off 
>>> explicitly. I would propose to change the default behaviour of directmode 
>>> here to off as directmode seems to be something very special to me that 
>>> should be explicitly enabled.
>>>
>>> Regards
>>>
>>> Rüdiger
>>>
 -Original Message-
 From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
 Sent: Sonntag, 11. Oktober 2015 19:54
 To: dev@httpd.apache.org
 Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
 loaded

 Ok, in ssl_engine_io.c, lines 1426+ I see a hint:

/* XXX: we could actually move ssl_io_filter_handshake to an
 * 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Yann Ylavic]

On Mon, Oct 12, 2015 at 11:13 AM, Rainer Jung  wrote:
>
> Since I started this thread: IMHO this discussion shouldn't stop/influence
> 2.4.17. mod_http2 is experimental, so even a change of defaults in early
> releases should be OK. And any exotic interop problems are not critical
> enough to roll another release.

+1, not a showstopper from my POV.

RE: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Plüm , Rüdiger , Vodafone Group]

How about the following patch?

This separates the triggering of the SSL Handshake and the possible reading of 
the 24 bytes:

Index: h2_h2.c
===   
--- h2_h2.c (revision 1707437)
+++ h2_h2.c (working copy)
@@ -154,7 +154,7 @@

 temp = apr_brigade_create(c->pool, c->bucket_alloc);
 status = ap_get_brigade(c->input_filters, temp,
-AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24);
+AP_MODE_INIT, APR_BLOCK_READ, 0);

 if (status == APR_SUCCESS) {
 if (h2_ctx_protocol_get(c)
@@ -171,23 +171,31 @@
 char *s = NULL;
 apr_size_t slen;

-apr_brigade_pflatten(temp, , , c->pool);
-if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
-ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
-  "h2_h2, direct mode detected");
-h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
+status = ap_get_brigade(c->input_filters, temp,
+AP_MODE_SPECULATIVE, 
APR_BLOCK_READ, 24);
+if (status == APR_SUCCESS) {
+apr_brigade_pflatten(temp, , , c->pool);
+if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
+ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
+  "h2_h2, direct mode detected");
+h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
+}
+else {
+ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
+  "h2_h2, not detected in %d bytes: 
%s",
+  (int)slen, s);
+}
 }
-else {
-ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
-  "h2_h2, not detected in %d bytes: %s",
-  (int)slen, s);
-}
 }
+else {
+ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
+  "h2_h2, error reading 24 bytes speculative");
+}
 }
 }
 else {
 ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
-  "h2_h2, error reading 24 bytes speculative");
+  "h2_h2, Failed to init connection");
 }
 apr_brigade_destroy(temp);
 }

This would still block in the non ssl case if directmode is not set to off 
explicitly. I would propose to change the default behaviour of directmode here 
to off as directmode seems to be something very special to me that should be 
explicitly enabled.

Regards

Rüdiger

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: Sonntag, 11. Oktober 2015 19:54
> To: dev@httpd.apache.org
> Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
> loaded
> 
> Ok, in ssl_engine_io.c, lines 1426+ I see a hint:
> 
> /* XXX: we could actually move ssl_io_filter_handshake to an
>  * ap_hook_process_connection but would still need to call it for
>  * AP_MODE_INIT for protocols that may upgrade the connection
>  * rather than have SSLEngine On configured.
>  */
> if ((status = ssl_io_filter_handshake(inctx->filter_ctx)) !=
> APR_SUCCESS) {
> return ssl_io_filter_error(f, bb, status);
> }
> 
> The handshake handling gets triggered on a filter, so it all happens on
> READs (or WRITEs), which explains why connection hooks do not see it. This
> is quite some code which looks like we do not want to touch for a "quick
> fix". Also, there is no quick fix inside mod_http2 as before handshake,
> the SNI might also not be there yet, thus the vhost cannot be determined.
> etc.
> 
> For the current release, I think we should leave it as it is and advise
> that current mod_http2 is incompatible with things like nntp.
> 
> For the next release, I'd like the server to perform the handshake at a
> defined time, so other modules can rely on protocols being selected and
> correct vhost being known before the first read/write.
> 
> //Stefan
> 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Stefan Eissing]

The AP_MODE_INIT triggers the handshake nicely. But the protocol switching 
still happens
on the first read. currently looking how to trigger that best.

The h2_task_input is only ever run on "pseudo" connections. But I agree that
it should pass init down the filter chain. 

//Stefan

> Am 12.10.2015 um 10:54 schrieb Yann Ylavic :
> 
> Also, I wonder if we don't need:
> 
> Index: modules/http2/h2_task_input.c
> ===
> --- modules/http2/h2_task_input.c(revision 1707888)
> +++ modules/http2/h2_task_input.c(working copy)
> @@ -116,7 +116,7 @@ apr_status_t h2_task_input_read(h2_task_input *inp
> }
> 
> if (mode == AP_MODE_INIT) {
> -return APR_SUCCESS;
> +return ap_get_brigade(f->c->input_filters, bb, mode, block, 
> readbytes);
> }
> 
> if (input->bb) {
> --
> 
> We shouldn't since h2_task_input_read() is called at NETWORK level
> (hence after mod_ssl which "eats" AP_MODE_INIT), but it looks more
> correct (as it would be in mod_ssl) since anyway the
> core_input_filter() can handle it.
> 
> 
> On Mon, Oct 12, 2015 at 10:42 AM, Yann Ylavic  wrote:
>> The key here is using AP_MODE_INIT first (where 0 make sense) before
>> AP_MODE_SPECULATIVE.
>> 
>> 
>> On Mon, Oct 12, 2015 at 10:36 AM, Stefan Eissing
>>  wrote:
>>> Yeah, I tried a 0 len read. Unfortunately, the core input filter is not 
>>> happy about that:
>>> 
>>> core_filters.c, line 231
>>>AP_DEBUG_ASSERT(readbytes > 0);
>>> 
 Am 12.10.2015 um 10:25 schrieb Plüm, Rüdiger, Vodafone Group 
 :
 
 How about the following patch?
 
 This separates the triggering of the SSL Handshake and the possible 
 reading of the 24 bytes:
 
 Index: h2_h2.c
 ===
 --- h2_h2.c (revision 1707437)
 +++ h2_h2.c (working copy)
 @@ -154,7 +154,7 @@
 
temp = apr_brigade_create(c->pool, c->bucket_alloc);
status = ap_get_brigade(c->input_filters, temp,
 -AP_MODE_SPECULATIVE, APR_BLOCK_READ, 24);
 +AP_MODE_INIT, APR_BLOCK_READ, 0);
 
if (status == APR_SUCCESS) {
if (h2_ctx_protocol_get(c)
 @@ -171,23 +171,31 @@
char *s = NULL;
apr_size_t slen;
 
 -apr_brigade_pflatten(temp, , , c->pool);
 -if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 24)) {
 -ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
 -  "h2_h2, direct mode detected");
 -h2_ctx_protocol_set(ctx, is_tls? "h2" : "h2c");
 +status = ap_get_brigade(c->input_filters, temp,
 +AP_MODE_SPECULATIVE, 
 APR_BLOCK_READ, 24);
 +if (status == APR_SUCCESS) {
 +apr_brigade_pflatten(temp, , , c->pool);
 +if ((slen >= 24) && !memcmp(H2_MAGIC_TOKEN, s, 
 24)) {
 +ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
 +  "h2_h2, direct mode detected");
 +h2_ctx_protocol_set(ctx, is_tls? "h2" : 
 "h2c");
 +}
 +else {
 +ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
 +  "h2_h2, not detected in %d 
 bytes: %s",
 +  (int)slen, s);
 +}
}
 -else {
 -ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c,
 -  "h2_h2, not detected in %d bytes: 
 %s",
 -  (int)slen, s);
 -}
}
 +else {
 +ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
 +  "h2_h2, error reading 24 bytes 
 speculative");
 +}
}
}
else {
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
 -  "h2_h2, error reading 24 bytes speculative");
 +  "h2_h2, Failed to init connection");
}
apr_brigade_destroy(temp);
}
 
 This would still block in the non ssl case if directmode is not set to off 
 explicitly. I would propose to change the default behaviour of directmode 
 here to off as directmode seems to be something very special to me 

RE: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Plüm , Rüdiger , Vodafone Group]

> -Original Message-
> From: Stefan Eissing [mailto:stefan.eiss...@greenbytes.de]
> Sent: Montag, 12. Oktober 2015 10:58
> To: dev@httpd.apache.org
> Subject: Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is
> loaded
> 
> The AP_MODE_INIT triggers the handshake nicely. But the protocol switching
> still happens
> on the first read. currently looking how to trigger that best.

Even in the ALPN case the protocol switching happens on the first read after 
the handshake?

Regards

Rüdiger

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Stefan Eissing]

It is default for some, others do not do it. 

No browser speaks h2c nowadays. 

I think reading bytes on a connection which is supposed to allow h2c traffic 
should be fine. Then one could argue if 24 bytes can always be expected...

Since we no longer enabled h2c by default in a server, I expect it to be fine. 

//Stefan

> Am 12.10.2015 um 11:12 schrieb Yann Ylavic :
> 
> That would be better, but still the doc says "This mode falls outside
> the RFC 7540 but has become widely implemented as it is very
> convenient for development and testing".
> Is this something used by real world h2 clients?
> 
> On Mon, Oct 12, 2015 at 11:09 AM, Stefan Eissing
>  wrote:
>> I plan to change it to only happen for servers, where h2/h2c is among 
>> configured protocols.
>> 
>>> Am 12.10.2015 um 11:07 schrieb Yann Ylavic :
>>> 
>>> On Sun, Oct 11, 2015 at 7:15 PM, Yann Ylavic  wrote:
 On Sun, Oct 11, 2015 at 7:11 PM, Stefan Eissing
  wrote:
> Don't think so. But loading the module should do no harm, I think. And it 
> does now.
 
 Isn't configuring H2Direct on which harms?
>>> 
>>> Didn't figure out "H2Direct on" was the default for non-TLS.
>>> I don't think it's a good idea, beyond the NNTP case, the admin should
>>> have to enable this by her/his own.
>> 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Rainer Jung]

Am 12.10.2015 um 11:09 schrieb Stefan Eissing:

I plan to change it to only happen for servers, where h2/h2c is among 
configured protocols.


Am 12.10.2015 um 11:07 schrieb Yann Ylavic :

On Sun, Oct 11, 2015 at 7:15 PM, Yann Ylavic  wrote:

On Sun, Oct 11, 2015 at 7:11 PM, Stefan Eissing
 wrote:

Don't think so. But loading the module should do no harm, I think. And it does 
now.


Isn't configuring H2Direct on which harms?


Didn't figure out "H2Direct on" was the default for non-TLS.
I don't think it's a good idea, beyond the NNTP case, the admin should
have to enable this by her/his own.


Since I started this thread: IMHO this discussion shouldn't 
stop/influence 2.4.17. mod_http2 is experimental, so even a change of 
defaults in early releases should be OK. And any exotic interop problems 
are not critical enough to roll another release.


Regards,

Rainer

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Yann Ylavic]

On Mon, Oct 12, 2015 at 11:12 AM, Yann Ylavic  wrote:
> Is this something used by real world h2 clients?

I meant browers :p

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Stefan Eissing]

I plan to change it to only happen for servers, where h2/h2c is among 
configured protocols.

> Am 12.10.2015 um 11:07 schrieb Yann Ylavic :
> 
> On Sun, Oct 11, 2015 at 7:15 PM, Yann Ylavic  wrote:
>> On Sun, Oct 11, 2015 at 7:11 PM, Stefan Eissing
>>  wrote:
>>> Don't think so. But loading the module should do no harm, I think. And it 
>>> does now.
>> 
>> Isn't configuring H2Direct on which harms?
> 
> Didn't figure out "H2Direct on" was the default for non-TLS.
> I don't think it's a good idea, beyond the NNTP case, the admin should
> have to enable this by her/his own.

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Yann Ylavic]

On Sun, Oct 11, 2015 at 7:15 PM, Yann Ylavic  wrote:
> On Sun, Oct 11, 2015 at 7:11 PM, Stefan Eissing
>  wrote:
>> Don't think so. But loading the module should do no harm, I think. And it 
>> does now.
>
> Isn't configuring H2Direct on which harms?

Didn't figure out "H2Direct on" was the default for non-TLS.
I don't think it's a good idea, beyond the NNTP case, the admin should
have to enable this by her/his own.

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Yann Ylavic]

That would be better, but still the doc says "This mode falls outside
the RFC 7540 but has become widely implemented as it is very
convenient for development and testing".
Is this something used by real world h2 clients?

On Mon, Oct 12, 2015 at 11:09 AM, Stefan Eissing
 wrote:
> I plan to change it to only happen for servers, where h2/h2c is among 
> configured protocols.
>
>> Am 12.10.2015 um 11:07 schrieb Yann Ylavic :
>>
>> On Sun, Oct 11, 2015 at 7:15 PM, Yann Ylavic  wrote:
>>> On Sun, Oct 11, 2015 at 7:11 PM, Stefan Eissing
>>>  wrote:
 Don't think so. But loading the module should do no harm, I think. And it 
 does now.
>>>
>>> Isn't configuring H2Direct on which harms?
>>
>> Didn't figure out "H2Direct on" was the default for non-TLS.
>> I don't think it's a good idea, beyond the NNTP case, the admin should
>> have to enable this by her/his own.
>

Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Jeff Trawick]

On Fri, Oct 9, 2015 at 1:40 PM, Jim Jagielski  wrote:

> The pre-release test tarballs for Apache httpd 2.4.17 can be found
> at the usual place:
>
> http://httpd.apache.org/dev/dist/
>
> I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.
>
> [ ] +1: Good to go
> [ ] +0: meh
> [ ] -1: Danger Will Robinson. And why.
>
> Vote will last the normal 72 hrs.
>
> NOTE: The *-deps are only there for convenience.
>
> Thx!
>
>
[X] +1: no regression, works with my existing production config with no
apparent problems

On Fedora 22, the only platform where I ran the test suite (latest version
as of yesterday afternoon), I'm getting a bunch of errors for http2 which I
don't see mentioned elsewhere.  I guess they're most likely triggered by
the Perl side of things???

t/modules/http2.t ... 1/52 # Failed test 11 in
t/modules/http2.t at line 174
# Failed test 12 in t/modules/http2.t at line 176
# Failed test 13 in t/modules/http2.t at line 174 fail #2
# Failed test 14 in t/modules/http2.t at line 176 fail #2
# Failed test 15 in t/modules/http2.t at line 162
# Failed test 16 in t/modules/http2.t at line 164
# Failed test 17 in t/modules/http2.t at line 162 fail #2
# Failed test 18 in t/modules/http2.t at line 164 fail #2
# Failed test 19 in t/modules/http2.t at line 174 fail #3
# Failed test 20 in t/modules/http2.t at line 176 fail #3
# Failed test 21 in t/modules/http2.t at line 162 fail #3
# Failed test 22 in t/modules/http2.t at line 164 fail #3
# Failed test 23 in t/modules/http2.t at line 162 fail #4
# Failed test 24 in t/modules/http2.t at line 164 fail #4
# Failed test 25 in t/modules/http2.t at line 162 fail #5
# Failed test 26 in t/modules/http2.t at line 164 fail #5
# Failed test 37 in t/modules/http2.t at line 174 fail #4
# Failed test 38 in t/modules/http2.t at line 176 fail #4
# Failed test 39 in t/modules/http2.t at line 174 fail #5
# Failed test 40 in t/modules/http2.t at line 176 fail #5
# Failed test 41 in t/modules/http2.t at line 162 fail #6
# Failed test 42 in t/modules/http2.t at line 164 fail #6
# Failed test 43 in t/modules/http2.t at line 162 fail #7
# Failed test 44 in t/modules/http2.t at line 164 fail #7
# Failed test 45 in t/modules/http2.t at line 174 fail #6
# Failed test 46 in t/modules/http2.t at line 176 fail #6
# Failed test 47 in t/modules/http2.t at line 162 fail #8
# Failed test 48 in t/modules/http2.t at line 164 fail #8
# Failed test 49 in t/modules/http2.t at line 162 fail #9
# Failed test 50 in t/modules/http2.t at line 164 fail #9
# Failed test 51 in t/modules/http2.t at line 162 fail #10
# Failed test 52 in t/modules/http2.t at line 164 fail #10
t/modules/http2.t ... Failed 32/52 subtests

(no other errors, DAV tests were skipped due to lack of appropriate Perl
module)

-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Rainer Jung]

Am 09.10.2015 um 19:40 schrieb Jim Jagielski:

The pre-release test tarballs for Apache httpd 2.4.17 can be found
at the usual place:

http://httpd.apache.org/dev/dist/

I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.

[X] +1: Good to go
[ ] +0: meh
[ ] -1: Danger Will Robinson. And why.

Vote will last the normal 72 hrs.


+1 to release and thank a bunch for RMing.
Hooray to HTTP/2.

In short: No regressions found.

Detailed report:

- Sigs and hashes OK
- contents of tarballs identical
- contents of tag and tarballs identical
  except for expected deltas
  (we could cleanup some m4 files in apr-util/xml/expat/conftools
   at the end of buildconf, no regression)

Built on

- Solaris 8+10 Sparc as 32 Bit Binaries
- SLES 10+11 (64 Bits)
- RHEL 6 (64 Bits)

For all platforms built

- with default (shared), shared and static modules
- with module sets none, few, most, all, reallyall and default
  (always mod_privileges disabled)
- using --enable-load-all-modules
- against "included" APR/APU from deps tarball,
  plus external APR/APU 1.5.2/1.5.4

- using external libraries
  - expat 2.1.0
  - pcre 8.37
  - openssl 1.0.2d
  - lua 5.2.4
  - distcache 1.5.1
  - libxml2 2.9.2
  - libnghttp2 1.3.4

- Tool chain:
- platform gcc except for Solaris
  (gcc 4.1.2 for Solaris 8 and 4.9.2 for Solaris 10)
- CFLAGS: -O2 -g -Wall -fno-strict-aliasing
  (and -mpcu=v9 on Solaris)

All builds succeeded
  - one compiler warning (no regression)
ssl/ssl_util_stapling.c:657: warning: 'ok' may be used uninitialized in 
this function


Tested for

- Solaris 8+10 (32), SLES 10+11 (64), RHEL 6 (64)
- MPMs prefork, worker, event
  (except event on Solaris8, unsupported)
- default, shared and static modules
- log levels info, debug and trace8
- module set reallyall (122 modules plus MPMs)
- Solaris 8 and SLES 10 still ongoing but so far OK

The following test failures were seen:

a Test 4 or 5 in t/modules/dav.t:
  Happens for 38 out of 319 runs (7 on SLES 10, 16 on SLES 11,
  13 on RHEL6, 2 on Solaris 10).
  Creation, modified and now times not in the correct order.
  This seems to be a system issue, all tests done on NFS,
  many tested on virtualized guests.
  Not a regression.

b Various tests in t/apache/expr_string.t: (6, 11, 14, 17, 20 ,23)
  Happens for 41 out of 319 runs (31 on SLES 10, 1 on SLES 11,
  9 on RHEL6).
  The failure is always on line 68, where the error_log contents
  are checked.
  Not a regression.

c Test 59 of t/modules/include.t only and always on
  Solaris.
  This is due to a bug in the test, which uses strftime()
  with a "%s" pattern that is not supported on Solaris.
  Until recently the server and the test client both returned
  verbatim "%s" and the test succeeded. After updating some
  Perl modules for the http2 tests, the perl client even
  on Solaris now supports "%s" in strftime and the test starts
  to fail. It seems we have to fix the test.

d Failure of t/modules/session.t on SlES 10.
  Warning: Use of "shift" without parentheses is ambiguous at line 39.
  Perl version is 5.8.8. The script uses a somewhat advances
  $x = shift // "NOTFOUND";
  construct, which seems to be ambiguous for old Perl (I had
  to look in the fine perl docs for the // operator).
  We could probably use a less elegant but more compatible
  construct instead.

e Failure of two tests in t/modules/http2.t.
  The tests do not run in a consistent order, so the test numbers don't
  help, but it always seems to be the "misdirected" test under SSL
  returning 404 instead of 421. Analysis by Yann indicates, that
  the test might be broken and needs some refinement to actually
  trigger a 421.

Regards,

Rainer

Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Eric Covener]

On Fri, Oct 9, 2015 at 1:40 PM, Jim Jagielski  wrote:
> I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.

+1

Tested on AIX 7.1/ppc64/xlc

After much head-scratching, compiling of new openssl/nghttp2, and lots
of myserious CPAN stuff, Test framework passed everything but the two
421 Misidireected testcases in http2.t already discussed elsewhere.

Michael Felt -- if you're listening, it would be nice if perzlr had
openssl 1.0.2 and nghttp2.  The latter compiled quite easily with just
-qcpluscmt.

Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Jim Jagielski]

A little over 4.5 hours until the VOTE closes...

test early, test often! :)

> On Oct 9, 2015, at 1:40 PM, Jim Jagielski  wrote:
> 
> The pre-release test tarballs for Apache httpd 2.4.17 can be found
> at the usual place:
> 
>   http://httpd.apache.org/dev/dist/
> 
> I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.
> 
> [ ] +1: Good to go
> [ ] +0: meh
> [ ] -1: Danger Will Robinson. And why.
> 
> Vote will last the normal 72 hrs.
> 
> NOTE: The *-deps are only there for convenience.
> 
> Thx!
> 

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Graham Leggett]

On 11 Oct 2015, at 7:00 PM, Stefan Eissing  wrote:

> Ok, analyzed the code. Here is what seems to be happening:
> 
> - mod_http2, in the connection hook, does a blocking, speculative read to
>  a) make sure the ALPN has been triggered

Looking at the code inside the event MPM that calls 
ap_run_process_connection(), it looks like you can just do a non blocking read, 
and if you haven’t received enough bytes, return DECLINED and expect to be 
called again.

This presupposes that other connection filters aren’t trying to be excessively 
cleaver by stealing your connection away from you and then responding to your 
data (for example by error-ing out), which they may do by doing blocking reads.

Regards,
Graham
—

Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Stefan Eissing]

+1

> Am 12.10.2015 um 15:13 schrieb Jim Jagielski :
> 
> A little over 4.5 hours until the VOTE closes...
> 
> test early, test often! :)
> 
>> On Oct 9, 2015, at 1:40 PM, Jim Jagielski  wrote:
>> 
>> The pre-release test tarballs for Apache httpd 2.4.17 can be found
>> at the usual place:
>> 
>>  http://httpd.apache.org/dev/dist/
>> 
>> I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.
>> 
>> [ ] +1: Good to go
>> [ ] +0: meh
>> [ ] -1: Danger Will Robinson. And why.
>> 
>> Vote will last the normal 72 hrs.
>> 
>> NOTE: The *-deps are only there for convenience.
>> 
>> Thx!
>> 
> 

Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Steffen]

All looks fine with building and running Win32/Win64 VC14/11/10

No regressions/issues seen.

Build with:

nghttp2 1.3.4 
apr 1.5.2 with IPv6 enabled 
apr-util 1.5.4 with Crypto OpenSSL enabled 
apr-iconv 1.2.1 
openssl 1.0.2d +asm 
zlib 1.2.8 +asm 
pcre 8.37 
httpd.exe with OPENSSL_Applink and VC14 SupportedOS Manifest 
libxml2 2.9.2 
lua 5.1.5 
expat 2.1.0



Steffen

-Original Message- 
From: Jim Jagielski

Sent: Friday, July 10, 2015 10:33 PM Newsgroups: gmane.comp.apache.devel
To: httpd
Subject: [VOTE] Release Apache httpd 2.4.16 as GA

The pre-release test tarballs for Apache httpd 2.4.16 can be found
at the usual place:

http://httpd.apache.org/dev/dist/

I'm calling a VOTE on releasing these as Apache httpd 2.4.16 GA.


-Original Message- 
From: Jim Jagielski 
Sent: Friday, October 9, 2015 7:40 PM 
To: httpd 
Subject: ** [VOTE] Release Apache httpd 2.4.17 as GA 


The pre-release test tarballs for Apache httpd 2.4.17 can be found
at the usual place:

http://httpd.apache.org/dev/dist/

I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.

[ ] +1: Good to go
[ ] +0: meh
[ ] -1: Danger Will Robinson. And why.

Vote will last the normal 72 hrs.

NOTE: The *-deps are only there for convenience.

Thx!

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Stefan Eissing]

With r1708107 I committed the following changes to /trunk:

mod_ssl:
- calling ap_switch_protocol directly after ap_select protocol from inside the 
SSL ALPN callback. Error in switching will result in a TLS error which seems 
correct. This makes sure that after the ALPN has been triggered, the protocol 
switch is in place.
- alpn_finished check in ssl_engine_io could be removed then. Less flags and 
#ifdefs overall...

mod_http2:
- connection hook triggers handshake by reading in AP_MODE_INIT 0 bytes
- connection hook only every tries to do a blocking read if H2Direct is enabled 
and the connection is still on protocol "http/1.1"
- the default for H2Direct has been changed to "off" for all kinds of 
connections
- documentation xml updated
- ht_task_input passes AP_MODE_INIT calls on to chained filters

I did not test the nntp case (can I? how?), but this should address the issue.

Observations:
- The AP_MODE_INIT read should be done in a connection hook of mod_ssl itself. 
Other modules like mod_http2 should not care about that.
- With SNI, the selected virtual host really is a property of the connection. 
It would be good to get access to it from other modules *without* having a 
request_rec

Thanks for all the good suggestiongs from Yann and Rüdiger!

//Stefan

> Am 12.10.2015 um 14:10 schrieb Graham Leggett :
> 
> On 11 Oct 2015, at 7:00 PM, Stefan Eissing  
> wrote:
> 
>> Ok, analyzed the code. Here is what seems to be happening:
>> 
>> - mod_http2, in the connection hook, does a blocking, speculative read to
>> a) make sure the ALPN has been triggered
> 
> Looking at the code inside the event MPM that calls 
> ap_run_process_connection(), it looks like you can just do a non blocking 
> read, and if you haven’t received enough bytes, return DECLINED and expect to 
> be called again.
> 
> This presupposes that other connection filters aren’t trying to be 
> excessively cleaver by stealing your connection away from you and then 
> responding to your data (for example by error-ing out), which they may do by 
> doing blocking reads.
> 
> Regards,
> Graham
> —
> 

[RESULT] Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Jim Jagielski]

The VOTE is CLOSED.

With more than the required 3 +1 (binding) votes, the VOTE
PASSES!

Thx to all testers and contributors. I will start the process
of pushing to the mirrors and will announce on Weds.

> On Oct 9, 2015, at 1:40 PM, Jim Jagielski  wrote:
> 
> The pre-release test tarballs for Apache httpd 2.4.17 can be found
> at the usual place:
> 
>   http://httpd.apache.org/dev/dist/
> 
> I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.
> 
> [ ] +1: Good to go
> [ ] +0: meh
> [ ] -1: Danger Will Robinson. And why.
> 
> Vote will last the normal 72 hrs.
> 
> NOTE: The *-deps are only there for convenience.
> 
> Thx!
> 

Re: [RESULT] Re: [VOTE] Release Apache httpd 2.4.17 as GA

[Stefan Eissing]

Yay! Thanks to all!

> Am 12.10.2015 um 19:41 schrieb Jim Jagielski :
> 
> The VOTE is CLOSED.
> 
> With more than the required 3 +1 (binding) votes, the VOTE
> PASSES!
> 
> Thx to all testers and contributors. I will start the process
> of pushing to the mirrors and will announce on Weds.
> 
>> On Oct 9, 2015, at 1:40 PM, Jim Jagielski  wrote:
>> 
>> The pre-release test tarballs for Apache httpd 2.4.17 can be found
>> at the usual place:
>> 
>>http://httpd.apache.org/dev/dist/
>> 
>> I'm calling a VOTE on releasing these as Apache httpd 2.4.17 GA.
>> 
>> [ ] +1: Good to go
>> [ ] +0: meh
>> [ ] -1: Danger Will Robinson. And why.
>> 
>> Vote will last the normal 72 hrs.
>> 
>> NOTE: The *-deps are only there for convenience.
>> 
>> Thx!
> 

Double 101 response on timeout with h2c/mod_http2

[Jacob Champion]

I was curious to see how the mod_reqtimeout filter removal worked with 
mod_http2, since mod_websocket needs something similar. So I opened up 
telnet and did a manual Upgrade: h2c with some junk HTTP2-Settings. I 
got the 101 Switching Protocols response and a binary chunk of SETTINGS, 
then let the connection sit there for a while.


60 seconds later, I got another 101 Switching Protocols response, this 
time with the typical HTTP/1.1 header block, and then the connection closed:



HTTP/1.1 101 Switching Protocols
Date: Mon, 12 Oct 2015 19:50:51 GMT
Server: Apache/2.4.18-dev (Unix) OpenSSL/1.0.2d
Content-Length: 0
Connection: close
Content-Type: text/html


From a short run with GDB, it looks like the "standard" output filter 
chain is not being disabled in this timeout case.


(Now that mod_http2 is (almost) released, if you'd prefer I move this to 
the bug tracker, just say the word.)


--Jacob

Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded

[Jacob Champion]

On 10/12/2015 02:12 AM, Yann Ylavic wrote:

That would be better, but still the doc says "[H2Direct] falls outside
the RFC 7540 but has become widely implemented as it is very
convenient for development and testing".


_Does_ it fall outside the RFC? Section 3.4 covers the establishment of 
(cleartext) HTTP/2 connections via prior knowledge, and seems to 
describe exactly this use case:


> A client MUST send the connection preface (Section 3.5) and then MAY
> immediately send HTTP/2 frames to such a server; servers can identify
> these connections by the presence of the connection preface.

I agree that the method of establishing this prior knowledge is 
undefined, but the h2-direct mode seems to be within the RFC's scope. If 
you agree, I'd suggest removing that line from the docs.


--Jacob