Skip to content

Commit 538bb67

Browse files
committed
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 538bb67

File tree

1 file changed

+71
-2
lines changed

1 file changed

+71
-2
lines changed

src/connector/builder.rs

Lines changed: 71 additions & 2 deletions
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,70 @@ 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", feature = "http2"))]
249+
fn test_builder() {
250+
let _connector = HttpsConnectorBuilder::new()
251+
.with_webpki_roots()
252+
.https_only()
253+
.enable_http1()
254+
.enable_http2()
255+
.build();
256+
}
257+
258+
#[test]
259+
#[cfg(all(feature = "http1", feature = "http2"))]
260+
#[should_panic(expected = "ALPN protocols should not be pre-defined")]
261+
fn test_reject_predefined_alpn() {
262+
let roots = rustls::RootCertStore::empty();
263+
let mut config_with_alpn = rustls::ClientConfig::builder()
264+
.with_safe_defaults()
265+
.with_root_certificates(roots)
266+
.with_no_client_auth();
267+
config_with_alpn.alpn_protocols = vec![b"fancyprotocol".to_vec()];
268+
let _connector = HttpsConnectorBuilder::new()
269+
.with_tls_config(config_with_alpn)
270+
.https_only()
271+
.enable_http1()
272+
.enable_http2()
273+
.build();
274+
}
275+
276+
#[test]
277+
#[cfg(all(feature = "http1", feature = "http2"))]
278+
fn test_alpn() {
279+
let roots = rustls::RootCertStore::empty();
280+
let tls_config = rustls::ClientConfig::builder()
281+
.with_safe_defaults()
282+
.with_root_certificates(roots)
283+
.with_no_client_auth();
284+
let connector = HttpsConnectorBuilder::new()
285+
.with_tls_config(tls_config.clone())
286+
.https_only()
287+
.enable_http1()
288+
.build();
289+
assert!(connector.tls_config.alpn_protocols.is_empty());
290+
let connector = HttpsConnectorBuilder::new()
291+
.with_tls_config(tls_config.clone())
292+
.https_only()
293+
.enable_http2()
294+
.build();
295+
assert_eq!(&connector.tls_config.alpn_protocols, &[b"h2".to_vec()]);
296+
let connector = HttpsConnectorBuilder::new()
297+
.with_tls_config(tls_config.clone())
298+
.https_only()
299+
.enable_http1()
300+
.enable_http2()
301+
.build();
302+
assert_eq!(
303+
&connector.tls_config.alpn_protocols,
304+
&[b"h2".to_vec(), b"http/1.1".to_vec()]
305+
);
306+
}
307+
}

0 commit comments

Comments
 (0)