Skip to content

Commit 6465e3a

Browse files
PawelStroinskiDerGuteMoritz
authored andcommitted
Lift ssl-context coercion to connection-pool fn
As suggested by @DerGuteMoritz in #728. This fixes the issue and makes the test added in the previous commit pass. To avoid a repeated coercion *also* in `http-connection` fn (which is an internal fn called only from `connection-pool` fn indirectly), the coercion is being removed from it. As a result, we have to rely solely on `connection-pool` fn for the coercion, where we don't know whether SSL context will be required or not. This includes a situation when a user hasn't supplied any `ssl-context` options, and we are still building an `SslContext`, not knowing whether actual requests will require it or not. This might look like a bit of potentially unnecessary work, but this hopefully won't affect anyone performance-wise, as it would be unusual to run `connection-pool` fn in a tight loop. On the positive side, all users will profit from the elimination of repeated `SslContext` building, whether they supply `ssl-context` options or not. Above all, having consistent code paths in both situations is nice. Notes: - `http-connection` fn has now always some `ssl-context` available regardless of `ssl?` flag, but the only place where it matters (`client/make-pipeline-builder`) already checks `ssl?` flag before making use of `ssl-context`, so this makes no difference. - All rich comments exercising `http-connection` fn with `ssl?` flag were updated, as now the caller has to supply `ssl-context` in that case.
1 parent 5fbdbb8 commit 6465e3a

File tree

4 files changed

+29
-35
lines changed

4 files changed

+29
-35
lines changed

src/aleph/http.clj

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,10 @@
231231
(when (and force-h2c? (not-any? #{:http2} http-versions))
232232
(throw (IllegalArgumentException. "force-h2c? may only be true when HTTP/2 is enabled."))))
233233

234-
(let [log-activity (:log-activity connection-options)
234+
(let [{:keys [log-activity
235+
http-versions
236+
insecure?]
237+
:or {http-versions [:http1]}} connection-options
235238
dns-options' (if-not (and (some? dns-options)
236239
(not (or (contains? dns-options :transport)
237240
(contains? dns-options :epoll?))))
@@ -244,7 +247,10 @@
244247
(assoc :name-resolver (netty/dns-resolver-group dns-options'))
245248

246249
(some? log-activity)
247-
(assoc :log-activity (netty/activity-logger "aleph-client" log-activity)))
250+
(assoc :log-activity (netty/activity-logger "aleph-client" log-activity))
251+
252+
true
253+
(update :ssl-context #(client/ssl-context % http-versions insecure?)))
248254
p (promise)
249255
create-pool-fn (or pool-builder-fn
250256
flow/instrumented-pool)

src/aleph/http/client.clj

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -689,22 +689,6 @@
689689

690690
resp))))
691691

692-
(defn- client-ssl-context
693-
"Returns a client SslContext, or nil if none is requested.
694-
Validates the ALPN setup."
695-
^SslContext
696-
[ssl? ssl-context http-versions insecure?]
697-
(if ssl?
698-
(if ssl-context
699-
(-> ssl-context
700-
(common/ensure-consistent-alpn-config http-versions)
701-
(netty/coerce-ssl-client-context))
702-
(let [ssl-ctx-opts {:application-protocol-config (netty/application-protocol-config http-versions)}]
703-
(if insecure?
704-
(netty/insecure-ssl-client-context ssl-ctx-opts)
705-
(netty/ssl-client-context ssl-ctx-opts))))
706-
nil))
707-
708692
(defn- setup-http1-client
709693
[{:keys [on-closed response-executor]
710694
:as opts}]
@@ -761,16 +745,13 @@
761745
bootstrap-transform
762746
name-resolver
763747
keep-alive?
764-
insecure?
765-
ssl-context
766748
ssl-endpoint-id-alg
767749
response-buffer-size
768750
epoll?
769751
transport
770752
proxy-options
771753
pipeline-transform
772754
log-activity
773-
http-versions
774755
force-h2c?
775756
on-closed
776757
connect-timeout]
@@ -784,7 +765,6 @@
784765
epoll? false
785766
name-resolver :default
786767
log-activity :debug
787-
http-versions [:http1]
788768
force-h2c? false}
789769
:as opts}]
790770

@@ -798,8 +778,6 @@
798778
(get proxy-options :keep-alive? true))))
799779
authority (str host (when explicit-port? (str ":" port)))
800780

801-
ssl-context (client-ssl-context ssl? ssl-context http-versions insecure?)
802-
803781
logger (cond
804782
(instance? LoggingHandler log-activity) log-activity
805783
(some? log-activity) (netty/activity-logger "aleph-client" log-activity)
@@ -810,7 +788,6 @@
810788
(assoc opts
811789
:proxy-connected proxy-connected
812790
:ssl? ssl?
813-
:ssl-context ssl-context
814791
:ssl-endpoint-id-alg ssl-endpoint-id-alg
815792
:remote-address remote-address
816793
:raw-stream? raw-stream?
@@ -868,7 +845,6 @@
868845
:raw-stream? raw-stream?
869846
:remote-address remote-address
870847
:response-buffer-size response-buffer-size
871-
:ssl-context ssl-context
872848
:ssl? ssl?)]
873849

874850
(log/debug (str "Using HTTP protocol: " protocol)
@@ -935,6 +911,19 @@
935911
:response-buffer-size response-buffer-size
936912
:t0 t0})))))))))))))))
937913

914+
(defn ssl-context
915+
"Coerces a client SSL context, including enforcement of its ALPN setup."
916+
(^SslContext [http-versions] (ssl-context nil http-versions false))
917+
(^SslContext [ssl-ctx http-versions insecure?]
918+
(if ssl-ctx
919+
(-> ssl-ctx
920+
(common/ensure-consistent-alpn-config http-versions)
921+
(netty/coerce-ssl-client-context))
922+
(let [ssl-ctx-opts {:application-protocol-config (netty/application-protocol-config http-versions)}]
923+
(if insecure?
924+
(netty/insecure-ssl-client-context ssl-ctx-opts)
925+
(netty/ssl-client-context ssl-ctx-opts))))))
926+
938927

939928

940929
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -970,7 +959,7 @@
970959
(InetSocketAddress/createUnresolved "www.google.com" (int 443))
971960
true
972961
{:on-closed #(println "http conn closed")
973-
:http-versions [:http1]}))
962+
:ssl-context (ssl-context [:http1])}))
974963

975964
(conn {:request-method :get}))
976965
)

src/aleph/http/http2.clj

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,7 +1426,7 @@
14261426
(InetSocketAddress. "postman-echo.com" (int 443))
14271427
true
14281428
{:on-closed #(log/debug "http conn closed")
1429-
:http-versions [:http2]}))
1429+
:ssl-context (aleph.http.client/ssl-context [:http2])}))
14301430

14311431
(def result @(conn {:request-method :get
14321432
;;:raw-stream? true
@@ -1438,7 +1438,7 @@
14381438
(InetSocketAddress. "postman-echo.com" (int 443))
14391439
true
14401440
{:on-closed #(log/debug "http conn closed")
1441-
:http-versions [:http2]}))
1441+
:ssl-context (aleph.http.client/ssl-context [:http2])}))
14421442

14431443
(let [body-string "Body test"
14441444
fpath (Files/createTempFile "test" ".txt" (into-array FileAttribute []))
@@ -1482,7 +1482,7 @@
14821482
(InetSocketAddress. "postman-echo.com" (int 443))
14831483
true
14841484
{:on-closed #(log/debug "http conn closed")
1485-
:http-versions [:http2]}))
1485+
:ssl-context (aleph.http.client/ssl-context [:http2])}))
14861486

14871487
(def result
14881488
@(conn {:request-method :post
@@ -1504,7 +1504,7 @@
15041504
(InetSocketAddress. "postman-echo.com" (int 443))
15051505
true
15061506
{:on-closed #(log/debug "http conn closed")
1507-
:http-versions [:http2]}))
1507+
:ssl-context (aleph.http.client/ssl-context [:http2])}))
15081508

15091509
(let [req {:request-method :post
15101510
:uri "/post"
@@ -1521,7 +1521,7 @@
15211521
(InetSocketAddress. "postman-echo.com" (int 443))
15221522
true
15231523
{:on-closed #(log/debug "http conn closed")
1524-
:http-versions [:http1]}))
1524+
:ssl-context (aleph.http.client/ssl-context [:http1])}))
15251525

15261526
(def result @(conn {:request-method :get
15271527
:uri "/gzip" #_ "/deflate"

src/aleph/http/server.clj

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@
885885
(InetSocketAddress. "127.0.0.1" (int port))
886886
true
887887
{:on-closed #(log/debug "http conn closed")
888-
:http-versions [:http2]}))
888+
:ssl-context (aleph.http.client/ssl-context [:http2])}))
889889

890890
(def result @(d/timeout! (conn {:request-method :get})
891891
2000
@@ -906,8 +906,7 @@
906906
(InetSocketAddress. "aleph.localhost" (int port))
907907
true
908908
{:on-closed #(log/debug "http conn closed")
909-
:http-versions [:http2]
910-
:insecure? true}))
909+
:ssl-context (aleph.http.client/ssl-context nil [:http2] true)}))
911910

912911
(def result @(d/timeout! (conn {:request-method :get})
913912
2000

0 commit comments

Comments
 (0)