jronak commented 2 years ago

This PR adds a connection sniffer, which is used in the following PR to sniff the initial bytes from the connection to identify the TLS connection.

codecov[bot] commented 2 years ago

Codecov Report

Merging #2133 (107142c) into dev (f25ebb4) will decrease coverage by 0.02%. The diff coverage is 87.50%.

@@            Coverage Diff             @@
##              dev    #2133      +/-   ##
==========================================
- Coverage   87.71%   87.69%   -0.03%     
==========================================
  Files         251      252       +1     
  Lines       14041    14057      +16     
==========================================
+ Hits        12316    12327      +11     
- Misses       1336     1339       +3     
- Partials      389      391       +2

Impacted Files	Coverage Δ
transport/internal/tlsmux/conn_sniffer.go	`87.50% <87.50%> (ø)`
transport/tchannel/peer.go	`93.42% <0.00%> (-3.95%)`	:arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f25ebb4...107142c. Read the comment docs.

jkanywhere commented 2 years ago

All your connSniffer tests use "test" as the data. I would appreciate some tests with longer data and the second call to Read being for more bytes or fewer bytes than the first call to Read.

optional: Since the purpose of tlsmux.connSniffer is to check for TLS, I think we should encapsulate that functionality:

func (c *connSniffer) isTLS() (bool, error) {
    return isTLSClientHelloRecord(c)
}

That is more of a stylistic choice and does not change the functionality.

yarpc / yarpc-go

[3/n] tlsmux: add connection sniffer #2133

Codecov Report