httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Eissing <stefan.eiss...@greenbytes.de>
Subject Re: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded
Date Mon, 12 Oct 2015 08:36:08 GMT
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.pluem@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.eissing@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
>> 


Mime
View raw message