Skip to content

Commit 17f1668

Browse files
g2pdjc
authored andcommitted
Fix bad assert on alpn_protocols and add tests
An assert was added on WantsProtocols1.wrap_connector which was the wrong place, as we update the alpn_protocols field ourselves before calling wrap_connector. Asserting on with_tls_config is sufficient, this is the only place ConnectorBuilder(WantsSchemes) is constructed and the builder will always pass here. Add tests for alpn_protocols and the expected panic when trying to force it from outside.
1 parent 82e8c21 commit 17f1668

File tree

1 file changed

+69
-2
lines changed

1 file changed

+69
-2
lines changed

src/connector/builder.rs

+69-2
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ impl ConnectorBuilder<WantsTlsConfig> {
4444
/// [`enable_http2`](ConnectorBuilder::enable_http2)) before the
4545
/// connector is built.
4646
pub fn with_tls_config(self, config: ClientConfig) -> ConnectorBuilder<WantsSchemes> {
47-
assert!(config.alpn_protocols.is_empty());
47+
assert!(
48+
config.alpn_protocols.is_empty(),
49+
"ALPN protocols should not be pre-defined"
50+
);
4851
ConnectorBuilder(WantsSchemes { tls_config: config })
4952
}
5053

@@ -129,7 +132,6 @@ pub struct WantsProtocols1 {
129132

130133
impl WantsProtocols1 {
131134
fn wrap_connector<H>(self, conn: H) -> HttpsConnector<H> {
132-
assert!(self.tls_config.alpn_protocols.is_empty());
133135
HttpsConnector {
134136
force_https: self.https_only,
135137
http: conn,
@@ -236,3 +238,68 @@ impl ConnectorBuilder<WantsProtocols3> {
236238
self.0.inner.wrap_connector(conn)
237239
}
238240
}
241+
242+
#[cfg(test)]
243+
mod tests {
244+
use super::ConnectorBuilder as HttpsConnectorBuilder;
245+
246+
// Typical usage
247+
#[test]
248+
#[cfg(all(feature = "webpki-roots", feature = "http1"))]
249+
fn test_builder() {
250+
let _connector = HttpsConnectorBuilder::new()
251+
.with_webpki_roots()
252+
.https_only()
253+
.enable_http1()
254+
.build();
255+
}
256+
257+
#[test]
258+
#[cfg(feature = "http1")]
259+
#[should_panic(expected = "ALPN protocols should not be pre-defined")]
260+
fn test_reject_predefined_alpn() {
261+
let roots = rustls::RootCertStore::empty();
262+
let mut config_with_alpn = rustls::ClientConfig::builder()
263+
.with_safe_defaults()
264+
.with_root_certificates(roots)
265+
.with_no_client_auth();
266+
config_with_alpn.alpn_protocols = vec![b"fancyprotocol".to_vec()];
267+
let _connector = HttpsConnectorBuilder::new()
268+
.with_tls_config(config_with_alpn)
269+
.https_only()
270+
.enable_http1()
271+
.build();
272+
}
273+
274+
#[test]
275+
#[cfg(all(feature = "http1", feature = "http2"))]
276+
fn test_alpn() {
277+
let roots = rustls::RootCertStore::empty();
278+
let tls_config = rustls::ClientConfig::builder()
279+
.with_safe_defaults()
280+
.with_root_certificates(roots)
281+
.with_no_client_auth();
282+
let connector = HttpsConnectorBuilder::new()
283+
.with_tls_config(tls_config.clone())
284+
.https_only()
285+
.enable_http1()
286+
.build();
287+
assert!(connector.tls_config.alpn_protocols.is_empty());
288+
let connector = HttpsConnectorBuilder::new()
289+
.with_tls_config(tls_config.clone())
290+
.https_only()
291+
.enable_http2()
292+
.build();
293+
assert_eq!(&connector.tls_config.alpn_protocols, &[b"h2".to_vec()]);
294+
let connector = HttpsConnectorBuilder::new()
295+
.with_tls_config(tls_config.clone())
296+
.https_only()
297+
.enable_http1()
298+
.enable_http2()
299+
.build();
300+
assert_eq!(
301+
&connector.tls_config.alpn_protocols,
302+
&[b"h2".to_vec(), b"http/1.1".to_vec()]
303+
);
304+
}
305+
}

0 commit comments

Comments
 (0)