From e881de2051242bf29ad54362c1c8d4eaf898f152 Mon Sep 17 00:00:00 2001 From: aircraft-cerier <58537442+aircraft-cerier@users.noreply.github.com> Date: Mon, 10 Feb 2020 12:44:25 -0800 Subject: [PATCH 1/4] Added case to stripPasswords function for mysql DSN --- xray/sql.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/xray/sql.go b/xray/sql.go index 83f8e396..1e20f6f1 100644 --- a/xray/sql.go +++ b/xray/sql.go @@ -280,6 +280,14 @@ func stripPasswords(dsn string) string { inBraces = false buf.UnreadByte() } + case '@': + isPassword = true + flush() + resLen := res.Len() + if res.Bytes()[resLen-1] == ':' { + res.Truncate(resLen - 1) + } + res.WriteByte(c) } } inBraces = false From c9f218362bae9e1b7c5a088975dae5ed585ea3d5 Mon Sep 17 00:00:00 2001 From: aircraft-cerier <58537442+aircraft-cerier@users.noreply.github.com> Date: Tue, 11 Feb 2020 09:52:17 -0800 Subject: [PATCH 2/4] adding MySQL connection string test case and cleanup --- xray/sql.go | 2 +- xray/sql_test.go | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/xray/sql.go b/xray/sql.go index 1e20f6f1..17927ac1 100644 --- a/xray/sql.go +++ b/xray/sql.go @@ -284,7 +284,7 @@ func stripPasswords(dsn string) string { isPassword = true flush() resLen := res.Len() - if res.Bytes()[resLen-1] == ':' { + if resLen > 0 && res.Bytes()[resLen-1] == ':' { res.Truncate(resLen - 1) } res.WriteByte(c) diff --git a/xray/sql_test.go b/xray/sql_test.go index 4680900e..fa6a6dd8 100644 --- a/xray/sql_test.go +++ b/xray/sql_test.go @@ -182,6 +182,16 @@ func (s *sqlTestSuite) TestSemicolonPasswordConnectionString() { s.Equal("", s.db.url) } +func (s *sqlTestSuite) TestMySQLConnectionString() { + s.mockDB("username:password@protocol(address:1234)/dbname?param=value") + s.mockMySQL(nil) + s.connect() + + s.Require().NoError(s.mock.ExpectationsWereMet()) + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) + s.Equal("", s.db.url) +} + func (s *sqlTestSuite) TestPSQL() { s.mockDB("") s.mockPSQL(nil) From bf5395f45c1bafee61739efa97e5a998babb33ca Mon Sep 17 00:00:00 2001 From: aircraft-cerier <58537442+aircraft-cerier@users.noreply.github.com> Date: Wed, 12 Feb 2020 13:25:06 -0800 Subject: [PATCH 3/4] fixing to cover MySQL use case for when no password is part of dsn. Also separating test cases for pre Go1.11 and Go 1.11 and later. the Go team made some changes to URL parsing as a security fix in 1.13 and backported the change to 1.11.13 and 1.12.8. --- xray/sql.go | 14 ++++++++------ xray/sql_go111_test.go | 31 +++++++++++++++++++++++++++++++ xray/sql_prego111_test.go | 31 +++++++++++++++++++++++++++++++ xray/sql_test.go | 10 ---------- 4 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 xray/sql_go111_test.go create mode 100644 xray/sql_prego111_test.go diff --git a/xray/sql.go b/xray/sql.go index 17927ac1..14125cac 100644 --- a/xray/sql.go +++ b/xray/sql.go @@ -281,13 +281,15 @@ func stripPasswords(dsn string) string { buf.UnreadByte() } case '@': - isPassword = true - flush() - resLen := res.Len() - if resLen > 0 && res.Bytes()[resLen-1] == ':' { - res.Truncate(resLen - 1) + if strings.Contains(res.String(), ":") { + resLen := res.Len() + if resLen > 0 && res.Bytes()[resLen-1] == ':' { + res.Truncate(resLen - 1) + } + isPassword = true + flush() + res.WriteByte(c) } - res.WriteByte(c) } } inBraces = false diff --git a/xray/sql_go111_test.go b/xray/sql_go111_test.go new file mode 100644 index 00000000..9562c23f --- /dev/null +++ b/xray/sql_go111_test.go @@ -0,0 +1,31 @@ +// Copyright 2017-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the License. A copy of the License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + +// +build go1.11 + +package xray + +func (s *sqlTestSuite) TestMySQLPasswordConnectionString() { + s.mockDB("username:password@protocol(address:1234)/dbname?param=value") + s.mockMySQL(nil) + s.connect() + + s.Require().NoError(s.mock.ExpectationsWereMet()) + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) + s.Equal("", s.db.url) +} + +func (s *sqlTestSuite) TestMySQLPasswordlessConnectionString() { + s.mockDB("username@protocol(address:1234)/dbname?param=value") + s.mockMySQL(nil) + s.connect() + + s.Require().NoError(s.mock.ExpectationsWereMet()) + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) + s.Equal("", s.db.url) +} diff --git a/xray/sql_prego111_test.go b/xray/sql_prego111_test.go new file mode 100644 index 00000000..13d20e55 --- /dev/null +++ b/xray/sql_prego111_test.go @@ -0,0 +1,31 @@ +// Copyright 2017-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the License. A copy of the License is located at +// +// http://aws.amazon.com/apache2.0/ +// +// or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + +// +build !go1.11 + +package xray + +func (s *sqlTestSuite) TestMySQLPasswordConnectionStringPreGo111() { + s.mockDB("username:password@protocol(address:1234)/dbname?param=value") + s.mockMySQL(nil) + s.connect() + + s.Require().NoError(s.mock.ExpectationsWereMet()) + s.Equal("", s.db.connectionString) + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.url) +} + +func (s *sqlTestSuite) TestMySQLPasswordlessConnectionStringPreGo111() { + s.mockDB("username@protocol(address:1234)/dbname?param=value") + s.mockMySQL(nil) + s.connect() + + s.Require().NoError(s.mock.ExpectationsWereMet()) + s.Equal("", s.db.connectionString) + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.url) +} diff --git a/xray/sql_test.go b/xray/sql_test.go index fa6a6dd8..4680900e 100644 --- a/xray/sql_test.go +++ b/xray/sql_test.go @@ -182,16 +182,6 @@ func (s *sqlTestSuite) TestSemicolonPasswordConnectionString() { s.Equal("", s.db.url) } -func (s *sqlTestSuite) TestMySQLConnectionString() { - s.mockDB("username:password@protocol(address:1234)/dbname?param=value") - s.mockMySQL(nil) - s.connect() - - s.Require().NoError(s.mock.ExpectationsWereMet()) - s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) - s.Equal("", s.db.url) -} - func (s *sqlTestSuite) TestPSQL() { s.mockDB("") s.mockPSQL(nil) From dbd138febb97fb01c70f23a7ab39a3748997cd81 Mon Sep 17 00:00:00 2001 From: aircraft-cerier <58537442+aircraft-cerier@users.noreply.github.com> Date: Wed, 19 Feb 2020 09:57:49 -0800 Subject: [PATCH 4/4] updating test case to accomodate for 1.11.x and 1.12.x Go Version differences in Travis --- xray/sql_go111_test.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/xray/sql_go111_test.go b/xray/sql_go111_test.go index 9562c23f..ea81e011 100644 --- a/xray/sql_go111_test.go +++ b/xray/sql_go111_test.go @@ -16,8 +16,14 @@ func (s *sqlTestSuite) TestMySQLPasswordConnectionString() { s.connect() s.Require().NoError(s.mock.ExpectationsWereMet()) - s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) - s.Equal("", s.db.url) + if s.db.connectionString != "" { + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) + s.Equal("", s.db.url) + } + if s.db.url != "" { + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.url) + s.Equal("", s.db.connectionString) + } } func (s *sqlTestSuite) TestMySQLPasswordlessConnectionString() { @@ -26,6 +32,12 @@ func (s *sqlTestSuite) TestMySQLPasswordlessConnectionString() { s.connect() s.Require().NoError(s.mock.ExpectationsWereMet()) - s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) - s.Equal("", s.db.url) + if s.db.connectionString != "" { + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.connectionString) + s.Equal("", s.db.url) + } + if s.db.url != "" { + s.Equal("username@protocol(address:1234)/dbname?param=value", s.db.url) + s.Equal("", s.db.connectionString) + } }