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
<stefan.eiss...@greenbytes.de> 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
>> <ruediger.pl...@vodafone.com>:
>>
>> 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, &s, &slen, 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, &s, &slen, 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
>>>
>
