feat(engine): make netxlite.utlsConn public #1018
Conversation
internal/netxlite/utls.go
Outdated
| // newConnUTLS returns a NewConn function for creating utlsConn instances. | ||
| func newConnUTLS(clientHello *utls.ClientHelloID) func(conn net.Conn, config *tls.Config) (TLSConn, error) { | ||
| // NewConnUTLS returns a NewConn function for creating UTLSConn instances. | ||
| func NewConnUTLS(clientHello *utls.ClientHelloID) func(conn net.Conn, config *tls.Config) (TLSConn, error) { |
There was a problem hiding this comment.
The name of this function was ~fine as long as it was private. Now that it is public, I am wondering why its name seems to imply that it creates a new connection, while instead it returns a function for creating a new connection. I suppose we can probably find a better name. Additionally, or instead of doing that, we could also consider keeping this function private. It seems to me an adapter to allow creating uTLS connections inside netxlite/tls.go and I think most users want instead to call NewConnUTLSWithHelloID. Regardless of what we choose to do, perhaps NewConnUTLS is the right name for NewConnUTLSWithHelloID?
There was a problem hiding this comment.
It seems to me an adapter to allow creating uTLS connections inside netxlite/tls.go and I think most users want instead to call NewConnUTLSWithHelloID.
I have the same understanding.
perhaps NewConnUTLS is the right name for NewConnUTLSWithHelloID?
Makes sense.
And I agree, the adapter should rather be private. I can't think of a better name for it though, but I'll think about it
There was a problem hiding this comment.
Maybe
NewConnUTLSWithHelloID --> NewUTLSConn,
NewConnUTLS --> newConnUTLS (as before)
There was a problem hiding this comment.
How about:
NewConnUTLSWithHelloID --> NewUTLSConn,
newConnUTLS --> newUTLSConnFactory
Not exactly as before but at least it removes the ambiguity of two function only differing in their first letter capitalization and doing a different business?
There was a problem hiding this comment.
Yes, I think that's good 👍
kelmenhorst
force-pushed
the
issue/2378
branch
from
d2c47a1 to
cc38abc
Compare
Checklist
Description
This PR exposes the
netxlite.utlsConnto the experiments by making it public (-->UTLSConn)echcheckcan usenetxlite.UTLSConnnow so we don't need to re-implement utlsConn there any longer.Note that
NewConnUTLSWithHelloIDreturns*UTLSConnnow, instead ofTLSConn, to make it more convenient for experiment developers.