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 >