Skip to content

Commit

Permalink
fix(transport): Propagate errors in tls_config instead of unwrap/panic (
Browse files Browse the repository at this point in the history
#385)

* Propagate errors in tls_config instead of unwrap

Ran into `tls_connector` failing and causing our app to panic and shutdown as it seems there wasn't any way to avoid panicking in `tls_config`.

So after talking to @LucioFranco briefly `tls_config` now returns a `Result` instead and propagates errors to the caller, where they can be handled.

* Fix compile warning when tls feature is disabled
  • Loading branch information
repi authored Jul 6, 2020
1 parent f085aba commit 3b9d6a6
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 17 deletions.
2 changes: 1 addition & 1 deletion examples/src/gcp/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
.domain_name("pubsub.googleapis.com");

let channel = Channel::from_static(ENDPOINT)
.tls_config(tls_config)
.tls_config(tls_config)?
.connect()
.await?;

Expand Down
2 changes: 1 addition & 1 deletion examples/src/tls/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
.domain_name("example.com");

let channel = Channel::from_static("http://[::1]:50051")
.tls_config(tls)
.tls_config(tls)?
.connect()
.await?;

Expand Down
2 changes: 1 addition & 1 deletion examples/src/tls/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let server = EchoServer::default();

Server::builder()
.tls_config(ServerTlsConfig::new().identity(identity))
.tls_config(ServerTlsConfig::new().identity(identity))?
.add_service(pb::echo_server::EchoServer::new(server))
.serve(addr)
.await?;
Expand Down
2 changes: 1 addition & 1 deletion examples/src/tls_client_auth/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
.identity(client_identity);

let channel = Channel::from_static("http://[::1]:50051")
.tls_config(tls)
.tls_config(tls)?
.connect()
.await?;

Expand Down
2 changes: 1 addition & 1 deletion examples/src/tls_client_auth/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
.client_ca_root(client_ca_cert);

Server::builder()
.tls_config(tls)
.tls_config(tls)?
.add_service(pb::echo_server::EchoServer::new(server))
.serve(addr)
.await?;
Expand Down
2 changes: 1 addition & 1 deletion interop/src/bin/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
ClientTlsConfig::new()
.ca_certificate(ca)
.domain_name("foo.test.google.fr"),
);
)?;
}

let channel = endpoint.connect().await?;
Expand Down
2 changes: 1 addition & 1 deletion interop/src/bin/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ async fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
let key = tokio::fs::read("interop/data/server1.key").await?;
let identity = Identity::from_pem(cert, key);

builder = builder.tls_config(ServerTlsConfig::new().identity(identity));
builder = builder.tls_config(ServerTlsConfig::new().identity(identity))?;
}

let test_service = server::TestServiceServer::new(server::TestService::default());
Expand Down
12 changes: 8 additions & 4 deletions tonic/src/transport/channel/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,15 @@ impl Endpoint {
/// Configures TLS for the endpoint.
#[cfg(feature = "tls")]
#[cfg_attr(docsrs, doc(cfg(feature = "tls")))]
pub fn tls_config(self, tls_config: ClientTlsConfig) -> Self {
Endpoint {
tls: Some(tls_config.tls_connector(self.uri.clone()).unwrap()),
pub fn tls_config(self, tls_config: ClientTlsConfig) -> Result<Self, Error> {
Ok(Endpoint {
tls: Some(
tls_config
.tls_connector(self.uri.clone())
.map_err(|e| Error::from_source(e))?,
),
..self
}
})
}

/// Set the value of `TCP_NODELAY` option for accepted connections. Enabled by default.
Expand Down
4 changes: 2 additions & 2 deletions tonic/src/transport/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
//! let mut channel = Channel::from_static("https://example.com")
//! .tls_config(ClientTlsConfig::new()
//! .ca_certificate(Certificate::from_pem(&cert))
//! .domain_name("example.com".to_string()))
//! .domain_name("example.com".to_string()))?
//! .timeout(Duration::from_secs(5))
//! .rate_limit(5, Duration::from_secs(1))
//! .concurrency_limit(256)
Expand Down Expand Up @@ -74,7 +74,7 @@
//!
//! Server::builder()
//! .tls_config(ServerTlsConfig::new()
//! .identity(Identity::from_pem(&cert, &key)))
//! .identity(Identity::from_pem(&cert, &key)))?
//! .concurrency_limit_per_connection(256)
//! .add_service(my_svc)
//! .serve(addr)
Expand Down
15 changes: 11 additions & 4 deletions tonic/src/transport/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ use incoming::TcpIncoming;
#[cfg(feature = "tls")]
pub(crate) use incoming::TlsStream;

#[cfg(feature = "tls")]
use crate::transport::Error;

use super::service::{Or, Routes, ServerIo, ServiceBuilderExt};
use crate::{body::BoxBody, request::ConnectionInfo};
use futures_core::Stream;
Expand Down Expand Up @@ -97,11 +100,15 @@ impl Server {
/// Configure TLS for this server.
#[cfg(feature = "tls")]
#[cfg_attr(docsrs, doc(cfg(feature = "tls")))]
pub fn tls_config(self, tls_config: ServerTlsConfig) -> Self {
Server {
tls: Some(tls_config.tls_acceptor().unwrap()),
pub fn tls_config(self, tls_config: ServerTlsConfig) -> Result<Self, Error> {
Ok(Server {
tls: Some(
tls_config
.tls_acceptor()
.map_err(|e| Error::from_source(e))?,
),
..self
}
})
}

/// Set the concurrency limit applied to on requests inbound per connection.
Expand Down

0 comments on commit 3b9d6a6

Please sign in to comment.