httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Plüm, Rüdiger, Vodafone Group <ruediger.pl...@vodafone.com>
Subject RE: 2.4.17 test failure for mod_nntp_like_ssl when mod_http2 is loaded
Date Mon, 12 Oct 2015 08:25:00 GMT
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