brudo commented on PR #23: URL: https://github.com/apache/activemq-nms-openwire/pull/23#issuecomment-1169116217
I can take on the work to fail fast on invalid SslProtocol value (rather than failing late, as in the original code, or not failing at all even when explicitly passed an invalid value, as I have it right now) and will include it in this PR shortly. I notice that SslContext is a private class, and that the SslContext(string) constructor is only called from the default SslContext() constructor, and there is no exposed property of type SslContext; only the string property SslProtocol. In light of this, I don't think the SslContext class needs to be touched. Rather, the validation belongs in the public SslProtocol setters in SslTransportFactory and also in SslTransport (where SslProtocol could be overridden after creating an SslTransport and before calling Start). It would be the same validation in both the factory and the transport. In the places where the sslProtocol _field_ is set from SslContext.GetCurrent().SslProtocol, the validation would not be run repeatedly, since that bypasses the public property. However, I am also changing the sslProtocol field from `private` to `internal` in SslTransport, so that SslTransportFactory can assign directly in DoCreateTransport, without having it revalidated (really a micro-optimization but trivial). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
