From faeb391b6910021824078b4496971f4925bd97d5 Mon Sep 17 00:00:00 2001
From: Moritz Heidkamp <moritz.heidkamp@bevuta.com>
Date: Thu, 13 Jun 2024 16:20:20 +0200
Subject: [PATCH] Allow custom `SslContext` without ALPN again for
 HTTP/1.1-only case

With the introduction of HTTP/2 support, Aleph started to require a matching ALPN config to be
present in custom `SslContext` objects. This needlessly broke existing HTTP/1.1-only uses. With this
change, we now allow custom `SslContext` objects without ALPN config again if only HTTP/1.1 is
desired (via the `http-versions` option). Since this still happens to be the default, existing uses
should just work again.

Fixes #727
---
 src/aleph/http/common.clj | 29 ++++++++++++++++++++---------
 test/aleph/http_test.clj  | 12 +++++++++---
 2 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/aleph/http/common.clj b/src/aleph/http/common.clj
index d6262471..7898401f 100644
--- a/src/aleph/http/common.clj
+++ b/src/aleph/http/common.clj
@@ -158,8 +158,9 @@
    If `ssl-context` is an options map and it does contain an `:application-protocol-config` key, its
    supported protocols are validated to match `desired-http-versions`.
 
-   Otherwise, if `ssl-context` is an `io.netty.handler.ssl.SslContext` instance, its supported
-   protocols are validated to match `desired-http-versions`."
+   Otherwise, if `ssl-context` is an `io.netty.handler.ssl.SslContext` instance, its ALPN config's
+   protocols are validated to match `desired-http-versions`. If it doesn't have an ALPN config,
+   `desired-http-versions` may only contain `:http1`."
   [ssl-context desired-http-versions]
   (when-let [unsupported-http-versions (seq (remove supported-http-versions desired-http-versions))]
     (throw (ex-info "Unsupported desired HTTP versions"
@@ -178,13 +179,23 @@
     nil
 
     :else
-    (do
-      (assert-consistent-alpn-config!
-       (some-> ^SslContext ssl-context
-               (.applicationProtocolNegotiator)
-               (.protocols))
-       desired-http-versions)
-      ssl-context)))
+    ;; NOTE: `SslContext` always has an `ApplicationProtocolNegotiator`, even if
+    ;; `.applicationProtocolConfig` was never invoked in the `SslContextBuilder`. In this case, its
+    ;; `.protocols` will be an empty collection, which we thus treat as if no ALPN config is
+    ;; present.
+    (if-let [protocols (some-> ^SslContext ssl-context
+                               (.applicationProtocolNegotiator)
+                               (.protocols)
+                               (seq))]
+      (do
+        (assert-consistent-alpn-config!
+         protocols
+         desired-http-versions)
+        ssl-context)
+      (if (= [:http1] desired-http-versions)
+        ssl-context
+        (throw (ex-info "No ALPN supplied, but requested non-HTTP/1 versions that require ALPN."
+                        {:desired-http-versions desired-http-versions}))))))
 
 (defn validate-http1-pipeline-transform
   "Checks that :pipeline-transform is not being used with HTTP/2, since Netty
diff --git a/test/aleph/http_test.clj b/test/aleph/http_test.clj
index 30778ef7..e5cd2173 100644
--- a/test/aleph/http_test.clj
+++ b/test/aleph/http_test.clj
@@ -1339,7 +1339,7 @@
     (catch Exception e
       e)))
 
-(deftest test-http-versions-config
+(deftest test-http-versions-server-config
   (testing "ssl-context as options map"
 
     (testing "with different HTTP versions in ALPN config"
@@ -1424,12 +1424,18 @@
                                           (netty/application-protocol-config [:http2 :http1])))})]
         (is (= :started result))))
 
-    (testing "with no ALPN config"
+    (testing "with no ALPN config but desiring HTTP/2"
       (let [result (try-start-server
                     {:http-versions [:http2 :http1]
                      :ssl-context test-ssl/server-ssl-context})]
         (is (instance? ExceptionInfo result))
-        (is (= "Some desired HTTP versions are not part of ALPN config." (ex-message result))))))
+        (is (= "No ALPN supplied, but requested non-HTTP/1 versions that require ALPN." (ex-message result)))))
+
+    (testing "with no ALPN config but desiring only HTTP/1"
+      (let [result (try-start-server
+                    {:http-versions [:http1]
+                     :ssl-context test-ssl/server-ssl-context})]
+        (is (= :started result)))))
 
   (testing "HTTP/2 without ssl-context"
     (let [result (try-start-server