Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

automatically set the tls.Config.ServerName if unset #4032

Merged
merged 1 commit into from Aug 16, 2023

Conversation

marten-seemann
Copy link
Member

@marten-seemann marten-seemann commented Aug 16, 2023

This is needed for:

  1. Verifying the server certificate
  2. Generating a cache key for the ClientSessionCache. If ServerName is unset, crypto/tls will discard the session ticket.

Also saves one cloning of the tls.Config when using DialAddr{Early}.

This is similar to what the standard library does: https://cs.opensource.google/go/go/+/refs/tags/go1.21.0:src/crypto/tls/tls.go;l=147-154

@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #4032 (022b36a) into master (bda01bc) will increase coverage by 0.09%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #4032      +/-   ##
==========================================
+ Coverage   82.85%   82.94%   +0.09%     
==========================================
  Files         147      147              
  Lines       14789    14781       -8     
==========================================
+ Hits        12253    12260       +7     
+ Misses       2036     2022      -14     
+ Partials      500      499       -1     
Files Changed Coverage Δ
client.go 58.12% <40.00%> (+0.55%) ⬆️
transport.go 67.55% <90.91%> (+4.11%) ⬆️

... and 2 files with indirect coverage changes

@marten-seemann marten-seemann merged commit ca3842d into master Aug 16, 2023
29 checks passed
@marten-seemann marten-seemann deleted the set-tls-config-servername branch August 18, 2023 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants