Implement ms_tls_config_schannel experiment#1844
Conversation
|
There are some notable changes from https://github.com/microsoft/go/blob/microsoft/release-branch.go1.23/patches/0016-implement-ms_tls_config_schannel-experiment.patch... any notes on that to make it easier to review? |
The functionality is the same, it has just been adapted to the code in master. |
dagood
left a comment
dagood
left a comment
There was a problem hiding this comment.
Essentially just formatting suggestions. 👍
| +package tls | ||
| + | ||
| +func isSchannelCipherSuite(id uint16) bool { | ||
| + return false |
There was a problem hiding this comment.
panic("unreachable") might more clearly match intent?
There was a problem hiding this comment.
This function can be called from files not gated behind the ms_tls_config_schannel build tag, in which case it has to return false.
There was a problem hiding this comment.
It could be called, but it isn't called from anywhere when the experiment isn't active, right?
I don't understand why false makes sense here--to me that means "this isn't an algorithm schannel supports", but in this case we don't even know what schannel would support. I could see true making just as much sense depending on what this func is actually used for. (But it isn't used in either way, so... no context clues.)
A comment could clear it up fine if false really seems best.
| + } | ||
| + }) | ||
| +} | ||
| +func TestRemoveCipherSuiteCurvePrefix(t *testing.T) { |
There was a problem hiding this comment.
Newline bewteen funcs, name doesn't match.
| +func TestRemoveCipherSuiteCurvePrefix(t *testing.T) { | |
| + | |
| +func TestRemoveCipherSuiteCurveSuffix(t *testing.T) { |
Co-authored-by: Davis Goodin <dagood@users.noreply.github.com>
* Implement ms_tls_config_schannel experiment * add ms_tls_config_schannel goexperiment to test matrix * fix test * Apply suggestions from code review Co-authored-by: Davis Goodin <dagood@users.noreply.github.com> * add newline --------- Co-authored-by: Davis Goodin <dagood@users.noreply.github.com> (cherry picked from commit f5fc235)
* Implement ms_tls_config_schannel experiment * add ms_tls_config_schannel goexperiment to test matrix * fix test * Apply suggestions from code review * add newline --------- (cherry picked from commit f5fc235) Co-authored-by: Davis Goodin <dagood@users.noreply.github.com>
* Implement ms_tls_config_schannel experiment * add ms_tls_config_schannel goexperiment to test matrix * fix test * Apply suggestions from code review * add newline --------- (cherry picked from commit f5fc235) Co-authored-by: Davis Goodin <dagood@users.noreply.github.com> (cherry picked from commit e4796b0)
…1844) (#1848) * Implement ms_tls_config_schannel experiment (#1844) (#1847) * Implement ms_tls_config_schannel experiment * add ms_tls_config_schannel goexperiment to test matrix * fix test * Apply suggestions from code review * add newline --------- (cherry picked from commit f5fc235) Co-authored-by: Davis Goodin <dagood@users.noreply.github.com> (cherry picked from commit e4796b0) * fix conflicts * fix conflicts --------- Co-authored-by: Davis Goodin <dagood@users.noreply.github.com>
4 participants
This PR forward-ports the
ms_tls_config_schannelfeature from Go 1.23 tomain.For https://github.com/microsoft/go-lab/issues/215.