Skip to content

Commit faeb391

Browse files
committed
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
1 parent 079cb2e commit faeb391

File tree

2 files changed

+29
-12
lines changed

2 files changed

+29
-12
lines changed

Diff for: src/aleph/http/common.clj

+20-9
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,9 @@
158158
If `ssl-context` is an options map and it does contain an `:application-protocol-config` key, its
159159
supported protocols are validated to match `desired-http-versions`.
160160
161-
Otherwise, if `ssl-context` is an `io.netty.handler.ssl.SslContext` instance, its supported
162-
protocols are validated to match `desired-http-versions`."
161+
Otherwise, if `ssl-context` is an `io.netty.handler.ssl.SslContext` instance, its ALPN config's
162+
protocols are validated to match `desired-http-versions`. If it doesn't have an ALPN config,
163+
`desired-http-versions` may only contain `:http1`."
163164
[ssl-context desired-http-versions]
164165
(when-let [unsupported-http-versions (seq (remove supported-http-versions desired-http-versions))]
165166
(throw (ex-info "Unsupported desired HTTP versions"
@@ -178,13 +179,23 @@
178179
nil
179180

180181
:else
181-
(do
182-
(assert-consistent-alpn-config!
183-
(some-> ^SslContext ssl-context
184-
(.applicationProtocolNegotiator)
185-
(.protocols))
186-
desired-http-versions)
187-
ssl-context)))
182+
;; NOTE: `SslContext` always has an `ApplicationProtocolNegotiator`, even if
183+
;; `.applicationProtocolConfig` was never invoked in the `SslContextBuilder`. In this case, its
184+
;; `.protocols` will be an empty collection, which we thus treat as if no ALPN config is
185+
;; present.
186+
(if-let [protocols (some-> ^SslContext ssl-context
187+
(.applicationProtocolNegotiator)
188+
(.protocols)
189+
(seq))]
190+
(do
191+
(assert-consistent-alpn-config!
192+
protocols
193+
desired-http-versions)
194+
ssl-context)
195+
(if (= [:http1] desired-http-versions)
196+
ssl-context
197+
(throw (ex-info "No ALPN supplied, but requested non-HTTP/1 versions that require ALPN."
198+
{:desired-http-versions desired-http-versions}))))))
188199

189200
(defn validate-http1-pipeline-transform
190201
"Checks that :pipeline-transform is not being used with HTTP/2, since Netty

Diff for: test/aleph/http_test.clj

+9-3
Original file line numberDiff line numberDiff line change
@@ -1339,7 +1339,7 @@
13391339
(catch Exception e
13401340
e)))
13411341

1342-
(deftest test-http-versions-config
1342+
(deftest test-http-versions-server-config
13431343
(testing "ssl-context as options map"
13441344

13451345
(testing "with different HTTP versions in ALPN config"
@@ -1424,12 +1424,18 @@
14241424
(netty/application-protocol-config [:http2 :http1])))})]
14251425
(is (= :started result))))
14261426

1427-
(testing "with no ALPN config"
1427+
(testing "with no ALPN config but desiring HTTP/2"
14281428
(let [result (try-start-server
14291429
{:http-versions [:http2 :http1]
14301430
:ssl-context test-ssl/server-ssl-context})]
14311431
(is (instance? ExceptionInfo result))
1432-
(is (= "Some desired HTTP versions are not part of ALPN config." (ex-message result))))))
1432+
(is (= "No ALPN supplied, but requested non-HTTP/1 versions that require ALPN." (ex-message result)))))
1433+
1434+
(testing "with no ALPN config but desiring only HTTP/1"
1435+
(let [result (try-start-server
1436+
{:http-versions [:http1]
1437+
:ssl-context test-ssl/server-ssl-context})]
1438+
(is (= :started result)))))
14331439

14341440
(testing "HTTP/2 without ssl-context"
14351441
(let [result (try-start-server

0 commit comments

Comments
 (0)