Github user shinrich commented on the pull request:
https://github.com/apache/trafficserver/pull/194#issuecomment-119261713
The SNI name check looks better. A couple comments remaining.
computeSSLTrace() might be getting called more than once per transaction.
You are probably better off pull the block that calls computeSSLTrace() and
setSSLTrace() into the if(this->ssl == NULL) block and call that after the
make_ssl_connection call. Save a bit of unnecessary work.
The other comment is about the client_vc and server_vc manipulations in
HttpSM. Your casts are not going to work in the H2 and SPDY cases. The
server_vc will be ok, but the client_vc is a PluginVC in those cases.
Fortunately, you don't de-reference the client_vc only dynamic_cast it, so the
code is safe, but your tracing will be off for SPDY/H2. I know that you are
working on those issues for another project, so the SPDY/H2 wiretracing support
can be updated later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
