grpc / grpc/grpc-rust

Abstraction Conflict: Custom Connector Allowed, but Internally Enforced to Use tonic's TlsConnector

Open
#2,360 5 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
12.5k
Forks
1.3k
Avg merge
4d 7h
Merged PRs (30d)
24

Description

Reopening issue #2323 as not sure why it was closed. This is still an issue for me. To summarize:

I think there's a conflict in the abstraction here. In the Endpoint, we provide the `connect_with_connector` method to support arbitrary `Connector` implementations, including allowing developers to use their own TLS implementations. However, in the implementation of the `Connector` struct, we enforce that HTTPS requests must use the `TlsConnector` provided by `tonic`.

https://github.com/hyperium/tonic/blob/207577cf3ce0dd18b2a2f0e9fae8c98584d9711c/tonic/src/transport/channel/endpoint.rs#L502

https://github.com/hyperium/tonic/blob/207577cf3ce0dd18b2a2f0e9fae8c98584d9711c/tonic/src/transport/channel/service/connector.rs#L51-L76

Then @LucioFranco responded:
> If you disable the tls feature then this check will go away right? So I think that would solve your issue of using a custom tls impl.

However, I just recently hit this issue, the problem with the proposed solution is that the tls feature offers more than just custom TLS impls. In my case, I want the tls feature to be enabled so that I can leverage the impl Connected for tokio_rustls::TlsStream trait impl - since I am using my own custom connector which makes a TlsStream.

My suggestion would be to add some kind of configuration here similar to what is done in [hyper_tls::HttpsConnector](https://docs.rs/hyper-tls/latest/hyper_tls/struct.HttpsConnector.html#method.https_only) - where enforcing https (or the converse in this case, enforcing tls config if scheme is https) can be turned on/off.

Otherwise, in order for me to use the proposed solution, I will have to create a thin wrapper over TlsStream which impls Connected to satisfy my requirements.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.