search

Vapor-Deprecated / tls-provider

An ssl provider that adds client and server ssl capability to a Vapor project.
6 stars 3 forks source link

Allow for TLSClientStream and TLSServerStream to be subclassed or extended. #6

Closed Prince2k3 closed 8 years ago

Prince2k3 commented 8 years ago

With TLSServerStream and TLSClientStream as final there is no way to extend the feature of the Stream.

Prince2k3 commented 8 years ago

@LoganWright I'm seeing why these methods are final. There is no way to change the client and server properties as well. 

public var client: Client.Type? {
        guard modes.contains(.client) else { return nil }
        return Engine.HTTPClient<TLSClientStream>.self
    }

    public var server: Server.Type? {
        guard modes.contains(.server) else { return nil }
        return Engine.HTTPServer<TLSServerStream, HTTPParser<HTTPRequest>, HTTPSerializer<HTTPResponse>>.self
    }

Still would be cool to have a way to change this without overriding thing to much.

tanner0101 commented 8 years ago

The best option here would probably be to make a wrapper class around these final classes.

loganwright commented 8 years ago

@tannernelson @Prince2k3 I don't see us getting huge performance gains from final here and I think subclassing will be common enough for certificates as we're currently architected that removing the final keyword from these would be easier to use for people.

What do you guys think?

Prince2k3 commented 8 years ago

@tannernelson @LoganWright I think we need to solve for what the vapor-tis is for exactly. Is it meant to be a base for tls like operations? if so, the current state of it is not friendly to changing the underpinning to fit the need of the developer. What I'm facing when creating a APNS Provider is that I can't give it a certificate based on a config location. There isn't away to do that. Sure I can subclass the TLSClientStream but there still isn't a way to specify a location of the certificate and its private file.

tanner0101 commented 8 years ago

@Prince2k3 it wasn't built with the idea of being a base in mind. This is a Provider package, so it's meant for people to be able to bring in TLS support for Server and Client easily.

I think bringing in the qutheory/tls package and creating a vapor-apns provider would be the best solution here.

Then, if you find an issue with the qutheory/tls package not being configurable enough (it should be) we can make PRs into qutheory/tls.