Skip to content

Conversation

julienschmidt
Copy link
Member

Fixes #25

Currently with Go 1.0 API in tests until Travis CI supports Go 1.1

@ghost ghost assigned julienschmidt May 31, 2013
@@ -21,15 +21,15 @@ func TestDSNParser(t *testing.T) {
out string
loc *time.Location
}{
{"username:password@protocol(address)/dbname?param=value", "&{user:username passwd:password net:protocol addr:address dbname:dbname params:map[param:value] loc:%p}", time.UTC},
Copy link
Member

Choose a reason for hiding this comment

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

I didn't spot the changes here. Should be ok :)

Copy link
Member Author

Choose a reason for hiding this comment

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

The config struct has 2 new fields

@arnehormann
Copy link
Member

LGTM for me, but m server doesn't have SSL built in, so I didn't test this. If you tested it, I'm fine with it.
If it's not built into Travis version of MySQL, we should document or detect that somehow so this stays supported and we don't regress by accident because the test is skipped.

@arnehormann
Copy link
Member

Thought about it...
We should make the ssl test explicit and never skip it.
If Travis doesn't include ssl enabled Mysql, we'll have to find another way. If it does, we should make sure this test is never skipped, not even when Travis changes the build so we get early warnings.
Maybe we should setup a different repo like "github.com/go-sql-driver/mysql-ssl-status" importing the driver and requiring an ssl connection so we can see if that fails and include the link to the build status here as "ssl status"?

@@ -254,6 +254,35 @@ func (mc *mysqlConn) writeAuthPacket() error {
// Charset [1 byte]
data[12] = mc.charset

// SSL Connection Request Packet
// http://dev.mysql.com/doc/internals/en/connection-phase.html#packet-Protocol::SSLRequest
if mc.cfg.tls != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

This if case is the only part that could break without being noticed by Travis. I don't think this is worth all the effort. I have 2 test systems with TLS enabled, this should be enough.

julienschmidt added a commit that referenced this pull request Jun 1, 2013
@julienschmidt julienschmidt merged commit 55a708b into master Jun 1, 2013
@julienschmidt julienschmidt deleted the tls branch June 1, 2013 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SSL/TLS-Encryption
2 participants