Skip to content
New issue

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

#

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

Already on GitHub? # to your account

detect IPv6 literals and wrap them in square brackets in GetDBUri #143

Merged
merged 1 commit into from
Aug 15, 2016
Merged

detect IPv6 literals and wrap them in square brackets in GetDBUri #143

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion go/mysql/connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package mysql

import (
"fmt"
"net"
)

// ConnectionConfig is the minimal configuration required to connect to a MySQL server
Expand Down Expand Up @@ -47,5 +48,11 @@ func (this *ConnectionConfig) Equals(other *ConnectionConfig) bool {
}

func (this *ConnectionConfig) GetDBUri(databaseName string) string {
return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s", this.User, this.Password, this.Key.Hostname, this.Key.Port, databaseName)
var ip = net.ParseIP(this.Key.Hostname)
if (ip != nil) && (ip.To4() == nil) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why not to test To16() instead of To4()?

Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to check ip != nil before calling ip.To4(); To4 will return nil as expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dgryski I believe the ParseIP function can return nil, hence the check

Copy link
Contributor

Choose a reason for hiding this comment

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

ParseIP can return nil, but it's a typed nil that you can still call methods on. Nil receivers are valid in Go.

https://play.golang.org/p/imTbiRbYmB

Copy link
Contributor

Choose a reason for hiding this comment

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

OMG

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To answer your original question, the Go standard library will pack an IPv4 address (with zeros) into a 16-byte slice if you call .To16 on it, so it's not a useful check.

It frustrates me immensely that the language doesn't ship with a .AddressFamily() discriminator or a .IsIpv4() function or a way to call the inet_pton function.

For your second question, it seems stylistically preferable to me to check ip != nil because that does something conceptually different – it checks whether the passed input is an IP address literal at all. I'm of the school that explicit is always better than implicit.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks for clarifying

// Wrap IPv6 literals in square brackets
return fmt.Sprintf("%s:%s@tcp([%s]:%d)/%s", this.User, this.Password, this.Key.Hostname, this.Key.Port, databaseName)
} else {
return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s", this.User, this.Password, this.Key.Hostname, this.Key.Port, databaseName)
}
}