maskit commented on pull request #6870: URL: https://github.com/apache/trafficserver/pull/6870#issuecomment-642992402
Thank you for reviewing, @shinrich . Actually I was going to ask you a couple of questions after passing all the CI tests. 1 .SNIAction(Continuation *cont, const Context &ctx) Do you recall why it receives a `Continuation` but not `SSLNetVConnection`? Currently it's always SSLNetVC so I assumed it expects some kind of `NetVConnection`. However, if it expects `Continuation` literally, I should probably keep the function signature and cast `TLSSNISupport` to `NetVConnection`. 2. Additional processes after PerformAction There are additional processes after calling PerformAction, ``` netvc->attributes = HttpProxyPort::TRANSPORT_BLIND_TUNNEL; ``` and ``` setTLSValidProtocols(s, netvc->protocol_mask, TLSValidProtocols::max_mask); ``` I assume these applies changes that are made during SNI actions, but it seems like these could be done in the actions themselves. Do you think we should keep them out of the actions? Since these require SSLNetVC, to remove `SSLNetVCAccess` from the callback, I may need `TunnelSupport` for the first one, and may need to add a function to `ALPNSupport` for the second one. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
