[PATCH] mod_ssl: merge ssl_hook_Translate into ssl_hook_ReadReq

[Joe Orton <jo...@redhat.com>]

Is there any good reason why the ssl_hook_Translate functionality really
has to be in a translate_name hook?  PR 34452 is a nice snafu where the
ssl-unclean-shutdown setting is not taking effect if using a reverse
proxy, because the mod_ssl translate_hook never gets run.  mod_proxy is
only one common module which will trigger this, of course...

(this is a regression since 2.0.49 where mod_ssl started doing proper
SSL shutdowns, but it looks like it would affect 1.3+mod_ssl 2.8 too)

Moving this code to the post_read_request hook seems like a clean
solution; making sure the hook runs after mod_setenvif's match_headers.

Am I missing anything?

Index: modules/ssl/ssl_private.h
===================================================================
--- modules/ssl/ssl_private.h	(revision 161430)
+++ modules/ssl/ssl_private.h	(working copy)
@@ -531,7 +531,6 @@
 apr_status_t ssl_init_ModuleKill(void *data);
 
 /*  Apache API hooks  */
-int          ssl_hook_Translate(request_rec *);
 int          ssl_hook_Auth(request_rec *);
 int          ssl_hook_UserCheck(request_rec *);
 int          ssl_hook_Access(request_rec *);
Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c	(revision 161430)
+++ modules/ssl/ssl_engine_kernel.c	(working copy)
@@ -30,6 +30,8 @@
                                             -- Unknown                */
 #include "ssl_private.h"
 
+static void ssl_configure_env(request_rec *r, SSLConnRec *sslconn);
+
 /*
  *  Post Read Request Handler
  */
@@ -84,7 +86,31 @@
     if ((ssl = sslconn->ssl)) {
         SSL_set_app_data2(ssl, r);
     }
+    else {
+        return DECLINED;
+    }
 
+    /*
+     * Log information about incoming HTTPS requests
+     */
+    if (r->server->loglevel >= APLOG_INFO && ap_is_initial_req(r)) {
+        ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
+                     "%s HTTPS request received for child %ld (server %s)",
+                     (r->connection->keepalives <= 0 ?
+                     "Initial (No.1)" :
+                     apr_psprintf(r->pool, "Subsequent (No.%d)",
+                                  r->connection->keepalives+1)),
+                     r->connection->id,
+                     ssl_util_vhostid(r->pool, r->server));
+    }
+
+    /* SetEnvIf ssl-*-shutdown flags can only be per-server,
+     * so they won't change across keepalive requests
+     */
+    if (sslconn->shutdown_type == SSL_SHUTDOWN_TYPE_UNSET) {
+        ssl_configure_env(r, sslconn);
+    }
+
     return DECLINED;
 }
 
@@ -126,41 +152,6 @@
 }
 
 /*
- *  URL Translation Handler
- */
-int ssl_hook_Translate(request_rec *r)
-{
-    SSLConnRec *sslconn = myConnConfig(r->connection);
-
-    if (!(sslconn && sslconn->ssl)) {
-        return DECLINED;
-    }
-
-    /*
-     * Log information about incoming HTTPS requests
-     */
-    if (r->server->loglevel >= APLOG_INFO && ap_is_initial_req(r)) {
-        ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
-                     "%s HTTPS request received for child %ld (server %s)",
-                     (r->connection->keepalives <= 0 ?
-                     "Initial (No.1)" :
-                     apr_psprintf(r->pool, "Subsequent (No.%d)",
-                                  r->connection->keepalives+1)),
-                     r->connection->id,
-                     ssl_util_vhostid(r->pool, r->server));
-    }
-
-    /* SetEnvIf ssl-*-shutdown flags can only be per-server,
-     * so they won't change across keepalive requests
-     */
-    if (sslconn->shutdown_type == SSL_SHUTDOWN_TYPE_UNSET) {
-        ssl_configure_env(r, sslconn);
-    }
-
-    return DECLINED;
-}
-
-/*
  *  Access Handler
  */
 int ssl_hook_Access(request_rec *r)
Index: modules/ssl/mod_ssl.c
===================================================================
--- modules/ssl/mod_ssl.c	(revision 161430)
+++ modules/ssl/mod_ssl.c	(working copy)
@@ -478,6 +478,10 @@
 
 static void ssl_register_hooks(apr_pool_t *p)
 {
+    /* ssl_hook_ReadReq needs to use the BrowserMatch settings so must
+     * run after mod_setenvif's post_read_request hook. */ 
+    static const char *pre_prr[] = { "mod_setenvif.c", NULL };
+
     ssl_io_filter_register(p);
 
     ap_hook_pre_connection(ssl_hook_pre_connection,NULL,NULL, APR_HOOK_MIDDLE);
@@ -487,12 +491,11 @@
     ap_hook_default_port  (ssl_hook_default_port,  NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_pre_config    (ssl_hook_pre_config,    NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_child_init    (ssl_init_Child,         NULL,NULL, APR_HOOK_MIDDLE);
-    ap_hook_translate_name(ssl_hook_Translate,     NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_check_user_id (ssl_hook_UserCheck,     NULL,NULL, APR_HOOK_FIRST);
     ap_hook_fixups        (ssl_hook_Fixup,         NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_access_checker(ssl_hook_Access,        NULL,NULL, APR_HOOK_MIDDLE);
     ap_hook_auth_checker  (ssl_hook_Auth,          NULL,NULL, APR_HOOK_MIDDLE);
-    ap_hook_post_read_request(ssl_hook_ReadReq,    NULL,NULL, APR_HOOK_MIDDLE);
+    ap_hook_post_read_request(ssl_hook_ReadReq, pre_prr,NULL, APR_HOOK_MIDDLE);
     ap_hook_insert_filter (ssl_hook_Insert_Filter, NULL,NULL, APR_HOOK_MIDDLE);
 /*    ap_hook_handler       (ssl_hook_Upgrade,       NULL,NULL, APR_HOOK_MIDDLE); */