changed Protocols default to http/1.1 only, updated documentation, changed ap_select_...

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1701005 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/docs/manual/mod/core.xml b/docs/manual/mod/core.xml
index 845583430340b9a99fd3acfad5d8515b169b4c9a..a1bfd581911cbe5482fe8491302ecbac557233e5 100644 (file)
--- a/docs/manual/mod/core.xml
+++ b/docs/manual/mod/core.xml
@@ -3714,6 +3714,7 @@ Protocol https
     <name>Protocols</name>
     <description>Protocols available for a server/virtual host</description>
     <syntax>Protocols <var>protocol</var> ...</syntax>
+    <default>Protocols http/1.1</default>
     <contextlist><context>server config</context><context>virtual host</context></contextlist>
     <compatibility>Only available from Apache 2.4.17 and later.</compatibility>
     
@@ -3722,15 +3723,16 @@ Protocol https
             server/virtual host. The list determines the allowed protocols
             a client may negotiate for this server/host.</p>
         
-        <p>You only need to set protocols if you want to limit the available
-            protocols for a server/host. By default, all supported protocols
-            are available to a client.</p>
+        <p>You need to set protocols if you want to extend the available
+            protocols for a server/host. By default, only the http/1.1 protocol
+            (which includes the compatibility with 1.0 and 0.9 clients) is
+            allowed.</p>
         
-        <p>For example, if you want to support only HTTP/1.1 for a server, even
-            though HTTP/2 is available, just specify this protocol only:</p>
+        <p>For example, if you want to support HTTP/2 for a server with TLS, 
+            specify:</p>
         
         <highlight language="config">
-            Protocols http/1.1
+            Protocols h2 http/1.1
         </highlight>
 
         <p>Valid protocols are <code>http/1.1</code> for http and https connections,
@@ -3755,7 +3757,7 @@ Protocol https
     <name>ProtocolsHonorOrder</name>
     <description>Protocols available for a server/virtual host</description>
     <syntax>ProtocolsHonorOrder On|Off</syntax>
-    <default>ProtocolsHonorOrder Off</default>
+    <default>ProtocolsHonorOrder On</default>
     <contextlist><context>server config</context><context>virtual host</context></contextlist>
     <compatibility>Only available from Apache 2.4.17 and later.</compatibility>
     
@@ -3763,12 +3765,13 @@ Protocol https
         <p>This directive specifies if the server should honor the order in which
         the <directive>Protocols</directive> directive lists protocols.</p>
         
-        <p>By default, a client supplies a list of supported protocols and the server
-            selects an available one from that list in the given order.</p>
+        <p>If configured Off, the client supplied list order of protocols has 
+            precedence over the order in the server configuration.</p>
         
-        <p>With <directive>ProtocolsHonorOrder</directive> set to <code>on</code>, the
-            client ordering does not matter and only the ordering in the server
-        settings influences the outcome of the protocol negotiation.</p>
+        <p>With <directive>ProtocolsHonorOrder</directive> set to <code>on</code> 
+            (default), the client ordering does not matter and only the ordering 
+            in the server settings influences the outcome of the protocol 
+            negotiation.</p>
         
     </usage>
     <seealso><directive module="core">Protocols</directive></seealso>

diff --git a/include/http_protocol.h b/include/http_protocol.h
index 06f32f12c939a38ff7920269cdbc6ce9e275037d..3867b3458d7e5324fc26ebce08826dff45a64202 100644 (file)
--- a/include/http_protocol.h
+++ b/include/http_protocol.h
@@ -786,13 +786,17 @@ AP_DECLARE_HOOK(const char *,protocol_get,(const conn_rec *c))
 /**
  * Select a protocol for the given connection and optional request. Will return
  * the protocol identifier selected which may be the protocol already in place
- * on the connection. The server may ignore the choices given.
+ * on the connection. The selected protocol will be NULL if non of the given
+ * choices could be agreed upon (e.g. no proposal as made).
+ *
+ * A special case is where the choices itself is NULL (instead of empty). In
+ * this case there are no restrictions imposed on protocol selection.
  *
  * @param c The current connection
  * @param r The current request or NULL
  * @param s The server/virtual host selected
  * @param choices A list of protocol identifiers, normally the clients whishes
- * @return The selected protocol
+ * @return The selected protocol or NULL if no protocol could be agreed upon
  */
 AP_DECLARE(const char *) ap_select_protocol(conn_rec *c, request_rec *r, 
                                             server_rec *s,

diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c
index 60865b7ae8b1a70b770574c6ca0f4511efa4ca61..b9abb196f9f86244a71bf8e1cae7e39323066dbd 100644 (file)
--- a/modules/ssl/ssl_engine_kernel.c
+++ b/modules/ssl/ssl_engine_kernel.c
@@ -2173,6 +2173,7 @@ int ssl_callback_alpn_select(SSL *ssl,
     conn_rec *c = (conn_rec*)SSL_get_app_data(ssl);
     SSLConnRec *sslconn = myConnConfig(c);
     apr_array_header_t *client_protos;
+    const char *proposed;
     size_t len;
     int i;
 
@@ -2210,8 +2211,9 @@ int ssl_callback_alpn_select(SSL *ssl,
      */
     init_vhost(c, ssl);
     
-    *out = (const unsigned char *)ap_select_protocol(c, NULL, sslconn->server, 
-                                                     client_protos);
+    proposed = (const unsigned char *)ap_select_protocol(c, NULL, sslconn->server, 
+                                                         client_protos);
+    *out = proposed? proposed : ap_get_protocol(c);
     len = strlen((const char*)*out);
     if (len > 255) {
         ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02840)

diff --git a/server/core.c b/server/core.c
index b16267bc771b218a4739dbeaa0bf1266d80294cb..7c752b1658b49cc05502bd0ff399b1baef64ccb7 100644 (file)
--- a/server/core.c
+++ b/server/core.c
@@ -433,6 +433,7 @@ static void *merge_core_dir_configs(apr_pool_t *a, void *basev, void *newv)
 static void *create_core_server_config(apr_pool_t *a, server_rec *s)
 {
     core_server_config *conf;
+    const char **np;
     int is_virtual = s->is_virtual;
 
     conf = (core_server_config *)apr_pcalloc(a, sizeof(core_server_config));
@@ -5303,7 +5304,7 @@ static int core_upgrade_handler(request_rec *r)
             if (offers && offers->nelts > 0) {
                 const char *protocol = ap_select_protocol(c, r, r->server,
                                                           offers);
-                if (strcmp(protocol, ap_get_protocol(c))) {
+                if (protocol && strcmp(protocol, ap_get_protocol(c))) {
                     ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02909)
                                   "Upgrade selects '%s'", protocol);
                     /* Let the client know what we are upgrading to. */

diff --git a/server/protocol.c b/server/protocol.c
index 014957adfadcaaf1450c285f5c579347bace6359..3abd4d22be48cbd1ee7d664b98329ec6637a22ef 100644 (file)
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -1982,53 +1982,77 @@ AP_DECLARE(const char *) ap_select_protocol(conn_rec *c, request_rec *r,
                                             apr_array_header_t *choices)
 {
     apr_pool_t *pool = r? r->pool : c->pool;
-    apr_array_header_t *proposals;
-    const char *protocol = NULL, *existing = ap_get_protocol(c);
     core_server_config *conf = ap_get_core_module_config(s->module_config);
+    const char *protocol = NULL, *existing = ap_get_protocol(c);;
+    apr_array_header_t *proposals;
 
     if (APLOGcdebug(c)) {
         const char *p = apr_array_pstrcat(pool, conf->protocols, ',');
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, 
                       "select protocol from %s, choices=%s for server %s", 
-                      p, apr_array_pstrcat(pool, choices, ','),
+                      p, choices?
+                      apr_array_pstrcat(pool, choices, ',') : "NULL",
                       s->server_hostname);
     }
     
-    proposals = apr_array_make(pool, choices->nelts+1, sizeof(char *));
+    proposals = apr_array_make(pool, choices? choices->nelts+1 : 5, 
+                               sizeof(char *));
     ap_run_protocol_propose(c, r, s, choices, proposals);
     
     if (proposals->nelts > 0) {
         int i;
-        apr_array_header_t *prefs = ((conf->protocols_honor_order > 0
-                                      && conf->protocols->nelts > 0)? 
-                                     conf->protocols : choices);
+        apr_array_header_t *prefs = NULL;
+        
+        /* Default for protocols_honor_order is 'on' or != 0 */
+        if (conf->protocols_honor_order == 0 && choices && choices->nelts > 0) {
+            prefs = choices;
+        }
+        else {
+            prefs = conf->protocols;
+        }
 
         /* If the existing protocol has not been proposed, but is a choice,
          * add it to the proposals implicitly.
          */
-        if (!ap_array_str_contains(proposals, existing) 
+        if (choices 
+            && !ap_array_str_contains(proposals, existing) 
             && ap_array_str_contains(choices, existing)) {
             APR_ARRAY_PUSH(proposals, const char*) = existing;
         }
-        
+
         /* Select the most preferred protocol */
         if (APLOGcdebug(c)) {
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, 
-                          "select protocol, proposals=%s preferences=%s", 
+                          "select protocol, proposals=%s preferences=%s configured=%s", 
                           apr_array_pstrcat(pool, proposals, ','),
-                          apr_array_pstrcat(pool, prefs, ','));
+                          apr_array_pstrcat(pool, prefs, ','),
+                          apr_array_pstrcat(pool, conf->protocols, ','));
         }
-        for (i = 0; i < proposals->nelts; ++i) {
-            const char *p = APR_ARRAY_IDX(proposals, i, const char *);
-            if (conf->protocols->nelts > 0 
-                && !ap_array_str_contains(conf->protocols, p)) {
-                /* not a permitted protocol here */
-                continue;
-            }
-            else if (!protocol 
-                     || (protocol_cmp(prefs, protocol, p) < 0)) {
-                /* none selected yet or this on has preference */
-                protocol = p;
+        if (conf->protocols->nelts <= 0) {
+            /* nothing configured, by default, we only allow http/1.1 here.
+             * For now...
+             */
+            return (ap_array_str_contains(proposals, AP_PROTOCOL_HTTP1)?
+                    AP_PROTOCOL_HTTP1 : NULL);
+        }
+        else {
+            for (i = 0; i < proposals->nelts; ++i) {
+                const char *p = APR_ARRAY_IDX(proposals, i, const char *);
+                if (conf->protocols->nelts <= 0 && !strcmp(AP_PROTOCOL_HTTP1, p)) {
+                    /* nothing configured, by default, we only allow http/1.1 here.
+                     * For now...
+                     */
+                    continue;
+                }
+                if (!ap_array_str_contains(conf->protocols, p)) {
+                    /* not a configured protocol here */
+                    continue;
+                }
+                else if (!protocol 
+                         || (protocol_cmp(prefs, protocol, p) < 0)) {
+                    /* none selected yet or this one has preference */
+                    protocol = p;
+                }
             }
         }
     }
@@ -2037,7 +2061,7 @@ AP_DECLARE(const char *) ap_select_protocol(conn_rec *c, request_rec *r,
                       protocol? protocol : "(none)");
     }
 
-    return protocol? protocol : existing;
+    return protocol;
 }
 
 AP_DECLARE(apr_status_t) ap_switch_protocol(conn_rec *c, request_rec *r,