Skip to content
This repository has been archived by the owner on Jan 6, 2024. It is now read-only.

implement a trait to abstract over async tls acceptors #7

Closed
wants to merge 1 commit into from

Conversation

jbr
Copy link

@jbr jbr commented Apr 30, 2021

This provides an implementation for a simple crate with a single trait, allowing server authors to accept either this crate or async-native-tls. See async-email/async-native-tls#30 for the equivalent PR over there. As mentioned over there, if this goes well, I'll take a look at a trait for client-side connectors.

Thanks!

Copy link
Collaborator

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very nervous about having the just released crate as the default public API.

I am not opposed to adding such an abstraction under a feature flag with a version suffix (like async-tls-acceptor01), but I feel that the API needs to be designed a bit more.

@@ -110,6 +110,18 @@ impl TlsAcceptor {
}
}

#[async_tls_acceptor::async_trait]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since TlsAcceptor::accept returns a named future, this is more inefficient than using TlsAcceptor::accept directly. I haven't tried to see if that's possible, but if the trait can provide an API something like the following, it will be efficient enough. (Since Output is already a different type, it is not a problem that the type of Future is different between implementations.)

impl<Input> async_tls_acceptor::Acceptor<Input> for TlsAcceptor
where
    Input: AsyncRead + AsyncWrite + Send + Sync + Unpin + 'static,
{
    type Output = server::TlsStream<Input>;
    type Error = std::io::Error;
    type Future = Accept<Input>;
    fn accept(&self, input: Input) -> Self::Future {
        TlsAcceptor::accept(&self, input)
    }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this is not possible on stable currently, as it requires GATs in order to express futures like this (for situations in which there is no named future, and the unnamed future is not static):

impl<Input> async_tls_acceptor::Acceptor<Input> for TlsAcceptor
where
    Input: AsyncRead + AsyncWrite + Send + Sync + Unpin + 'static,
{
    type Output = TlsStream<Input>;
    type Error = crate::Error;
    type Future<'a> = Pin<Box<dyn Future<Output = Result<Self::Output, Self::Error>> + Send + 'a>>;

    fn accept<'a>(&'a self, input: Input) -> Self::Future<'a> {
        Box::pin(TlsAcceptor::accept(&self, input))
    }
}

Please correct me if I'm mistaken. Boxing the future doesn't seem like a terribly onerous price to pay until async traits land

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can work around that by making the lifetime a parameter of the trait itself. Hint: https://ytrizja.de/blog/2020/1001_rustiit4r.html

@taiki-e
Copy link
Collaborator

taiki-e commented Aug 6, 2023

I'm going to close this for now as there does not seem to be any other stakeholder interested in working on this abstraction other than you (I see no reactions in other tls related crates where you have submitted PRs).

Thanks anyway for the PR.

@taiki-e taiki-e closed this Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants