maskit commented on code in PR #8972:
URL: https://github.com/apache/trafficserver/pull/8972#discussion_r926400523
##########
iocore/net/ALPNSupport.cc:
##########
@@ -144,3 +144,36 @@ ALPNSupport::registerNextProtocolSet(SSLNextProtocolSet
*s, const SessionProtoco
this->npnSet = s;
npnSet->create_npn_advertisement(protoenabled, &npn, &npnsz);
}
+
+bool
+ALPNSupport::process_alpn_protocols(std::string_view protocols, unsigned char
*client_alpn_protocols, int &alpn_array_len)
Review Comment:
It would be nice if you could put this into some module that parses setting
values. Maybe RecHttp.cc, which parses server_ports.
##########
doc/admin-guide/files/records.config.en.rst:
##########
@@ -3956,6 +3956,12 @@ Client-Related Configuration
Enables (``1``) or disables (``0``) TLSv1_3 in the ATS client context. If
not specified, enabled by default
+.. ts:cv:: CONFIG proxy.config.ssl.client.alpn_protocols STRING ""
Review Comment:
I'm not sure if users want to set "ALPN string".
The ALPN protocol IDs are inconsistent; The ID for HTTP/1.1 is "http/1.1"
but not "h1.1" where ones for HTTP/2 and 3 are "h2" and "h3". I don't think
these IDs are user friendly.
Even if we use those raw IDs, the doc should say what IDs are available.
Obviously we can't set "spdy/1", although it's a registered valid ID. We
shouldn't send "h3" on a regular TLS connection, and similarly we shouldn't
send "h2" on a QUIC connection.
Also, the order of IDs is unclear if a user is unfamiliar with ALPN.
I don't have a good idea right now, but I think we need discussion about
this setting at minimum. Changing this setting would be an incompatible change
and we wouldn't be able to change it until 11.0 once we make 10.0 release with
this setting.
##########
proxy/ProxySession.cc:
##########
@@ -313,3 +315,21 @@ ProxySession::support_sni() const
{
return _vc ? _vc->support_sni() : false;
}
+
+PoolableSession *
+ProxySession::protocol_creation(NetVConnection *netvc)
+{
+ // Figure out what protocol was negotiated
+ int proto_index = SessionProtocolNameRegistry::INVALID;
+ SSLNetVConnection *sslnetvc = dynamic_cast<SSLNetVConnection *>(netvc);
Review Comment:
I don't see any reason to receive a NetVC here. Just a protocol id is
enough. Let's deal with NetVC on HttpSM side.
How about this? I want function names to have verbs.
```
PoolableSession *
ProxySession::create_outbound_sesion(int protocol_id) {
auto iter = ProtocolSessionCreateMap.find(protocol_id);
ink_release_assert(iter != ProtocolSessionCreateMap.end());
return iter->second();
}
```
--
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]
