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

Upgrades to Go 1.7 and fixes vet finding and TLS behavior change. #2281

Merged
merged 2 commits into from Nov 8, 2016

Conversation

slackpad
Copy link
Contributor

All unit tests pass but I want to chase down an issue the the TLS config before we merge. See comment below.

@@ -164,9 +197,9 @@ func (c *Config) OutgoingTLSWrapper() (DCWrapper, error) {
// Generate the wrapper based on hostname verification
if c.VerifyServerHostname {
wrapper := func(dc string, conn net.Conn) (net.Conn, error) {
conf := *tlsConfig
conf := clone(tlsConfig)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was required to fix a go vet finding since this was copying the state of an unexported mutex.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slackpad You might want to add a comment that in go1.8 tls.Config.Clone() will be exported.

golang/go@d24f446

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@magiconair thanks for the heads up on that!

@slackpad slackpad mentioned this pull request Sep 5, 2016
@slackpad slackpad added this to the 0.7.1 milestone Oct 25, 2016
@slackpad slackpad added the type/enhancement Proposed improvement or new feature label Oct 25, 2016
@slackpad slackpad force-pushed the f-go-1.7 branch 3 times, most recently from 30ea30d to ca98ddd Compare November 8, 2016 00:05
…on errors.

We traced through and realized that golang/go#15709
causes the output from the client to get buffered, which cuts off the alert
feedback due to the flush() call getting bypassed by the error return.
@slackpad slackpad merged commit 6de74c6 into master Nov 8, 2016
@slackpad slackpad deleted the f-go-1.7 branch November 8, 2016 02:15
@freddygv freddygv mentioned this pull request Jul 21, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants