Skip to content

Commit

Permalink
Allow custom SslContext without ALPN again for HTTP/1.1-only case
Browse files Browse the repository at this point in the history
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
  • Loading branch information
DerGuteMoritz committed Jun 13, 2024
1 parent 079cb2e commit 41f1a8c
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
29 changes: 20 additions & 9 deletions src/aleph/http/common.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
12 changes: 9 additions & 3 deletions test/aleph/http_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 41f1a8c

Please sign in to comment.