From 8a9f4a17af7ac1f7300b909fdbe7ded21fd6e16c Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 31 Jul 2018 21:52:05 +0800 Subject: [PATCH 1/6] privilege/privileges: sort user records in privilege cache --- privilege/privileges/cache.go | 98 +++++++++++++++++++++++++++--- privilege/privileges/cache_test.go | 37 +++++++++++ 2 files changed, 127 insertions(+), 8 deletions(-) diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go index 5cfaac646bd86..2dd8f02ba1949 100644 --- a/privilege/privileges/cache.go +++ b/privilege/privileges/cache.go @@ -15,6 +15,7 @@ package privileges import ( "fmt" + "sort" "strings" "sync/atomic" "time" @@ -47,7 +48,8 @@ func computePrivMask(privs []mysql.PrivilegeType) mysql.PrivilegeType { return mask } -type userRecord struct { +// UserRecord is used to represent a user record in privilege cache. +type UserRecord struct { Host string // max length 60, primary key User string // max length 16, primary key Password string // max length 41 @@ -103,7 +105,7 @@ type columnsPrivRecord struct { // MySQLPrivilege is the in-memory cache of mysql privilege tables. type MySQLPrivilege struct { - User []userRecord + User []UserRecord DB []dbRecord TablesPriv []tablesPrivRecord ColumnsPriv []columnsPrivRecord @@ -154,7 +156,87 @@ func noSuchTable(err error) bool { // LoadUserTable loads the mysql.user table from database. func (p *MySQLPrivilege) LoadUserTable(ctx sessionctx.Context) error { - return p.loadTable(ctx, "select Host,User,Password,Select_priv,Insert_priv,Update_priv,Delete_priv,Create_priv,Drop_priv,Process_priv,Grant_priv,References_priv,Alter_priv,Show_db_priv,Super_priv,Execute_priv,Index_priv,Create_user_priv,Trigger_priv from mysql.user order by host, user;", p.decodeUserTableRow) + err := p.loadTable(ctx, "select Host,User,Password,Select_priv,Insert_priv,Update_priv,Delete_priv,Create_priv,Drop_priv,Process_priv,Grant_priv,References_priv,Alter_priv,Show_db_priv,Super_priv,Execute_priv,Index_priv,Create_user_priv,Trigger_priv from mysql.user;", p.decodeUserTableRow) + if err != nil { + return errors.Trace(err) + } + // See https://dev.mysql.com/doc/refman/8.0/en/connection-access.html + // When multiple matches are possible, the server must determine which of them to use. It resolves this issue as follows: + // 1. Whenever the server reads the user table into memory, it sorts the rows. + // 2. When a client attempts to connect, the server looks through the rows in sorted order. + // 3. The server uses the first row that matches the client host name and user name. + // The server uses sorting rules that order rows with the most-specific Host values first. + p.SortUserTable() + return nil +} + +type sortedUserRecord []UserRecord + +func (s sortedUserRecord) Len() int { + return len([]UserRecord(s)) +} + +func (s sortedUserRecord) Less(i, j int) bool { + x := ([]UserRecord(s))[i] + y := ([]UserRecord(s))[j] + + // Compare two item by user's host first. + c1 := compareHost(x.Host, y.Host) + if c1 < 0 { + return true + } + if c1 > 0 { + return false + } + + // Then, compare item by user's name value. + return x.User < y.User +} + +// compareHost compares two host string using some special rule, return value 1, 0, -1 means > = <. +func compareHost(x, y string) int { + // The more-specific, the smaller it is. + // The pattern '%' means “any host” and is least specific. + if y == "%" { + if x == "%" { + return 0 + } + return -1 + } + + // The empty string '' also means “any host” but sorts after '%'. + if y == "" { + if x == "" { + return 0 + } + return -1 + } + + if strings.HasSuffix(y, "%") { + if !strings.HasSuffix(x, "%") { + return -1 + } + return 0 + } + + // For other case, the order is nondeterministic. + switch x < y { + case true: + return -1 + case false: + return 1 + } + return 0 +} + +func (s sortedUserRecord) Swap(i, j int) { + s1 := []UserRecord(s) + s1[i], s1[j] = s1[j], s1[i] +} + +// SortUserTable sorts p.User in the MySQLPrivilege struct. +func (p MySQLPrivilege) SortUserTable() { + sort.Sort(sortedUserRecord(p.User)) } // LoadDBTable loads the mysql.db table from database. @@ -206,7 +288,7 @@ func (p *MySQLPrivilege) loadTable(sctx sessionctx.Context, sql string, } func (p *MySQLPrivilege) decodeUserTableRow(row chunk.Row, fs []*ast.ResultField) error { - var value userRecord + var value UserRecord for i, f := range fs { switch { case f.ColumnAsName.L == "user": @@ -326,7 +408,7 @@ func decodeSetToPrivilege(s types.Set) mysql.PrivilegeType { return ret } -func (record *userRecord) match(user, host string) bool { +func (record *UserRecord) match(user, host string) bool { return record.User == user && patternMatch(host, record.patChars, record.patTypes) } @@ -355,7 +437,7 @@ func patternMatch(str string, patChars, patTypes []byte) bool { } // connectionVerification verifies the connection have access to TiDB server. -func (p *MySQLPrivilege) connectionVerification(user, host string) *userRecord { +func (p *MySQLPrivilege) connectionVerification(user, host string) *UserRecord { for i := 0; i < len(p.User); i++ { record := &p.User[i] if record.match(user, host) { @@ -365,7 +447,7 @@ func (p *MySQLPrivilege) connectionVerification(user, host string) *userRecord { return nil } -func (p *MySQLPrivilege) matchUser(user, host string) *userRecord { +func (p *MySQLPrivilege) matchUser(user, host string) *UserRecord { for i := 0; i < len(p.User); i++ { record := &p.User[i] if record.match(user, host) { @@ -557,7 +639,7 @@ func (p *MySQLPrivilege) UserPrivilegesTable() [][]types.Datum { return rows } -func appendUserPrivilegesTableRow(rows [][]types.Datum, user userRecord) [][]types.Datum { +func appendUserPrivilegesTableRow(rows [][]types.Datum, user UserRecord) [][]types.Datum { var isGrantable string if user.Privileges&mysql.GrantPriv > 0 { isGrantable = "YES" diff --git a/privilege/privileges/cache_test.go b/privilege/privileges/cache_test.go index 941f51f6353ee..a6c844d8672a5 100644 --- a/privilege/privileges/cache_test.go +++ b/privilege/privileges/cache_test.go @@ -261,3 +261,40 @@ func (s *testCacheSuite) TestAbnormalMySQLTable(c *C) { err = p.LoadAll(se) c.Assert(err, IsNil) } + +func (s *testCacheSuite) TestSortUserTable(c *C) { + var p privileges.MySQLPrivilege + p.User = []privileges.UserRecord{ + {Host: "%", User: "root"}, + {Host: "%", User: "jeffrey"}, + {Host: "localhost", User: "root"}, + {Host: "localhost", User: ""}, + } + p.SortUserTable() + result := []privileges.UserRecord{ + {Host: "localhost", User: "root"}, + {Host: "localhost", User: ""}, + {Host: "%", User: "jeffrey"}, + {Host: "%", User: "root"}, + } + checkUserRecord(p.User, result, c) + + p.User = []privileges.UserRecord{ + {Host: "%", User: "jeffrey"}, + {Host: "h1.example.net", User: ""}, + } + p.SortUserTable() + result = []privileges.UserRecord{ + {Host: "h1.example.net", User: ""}, + {Host: "%", User: "jeffrey"}, + } + checkUserRecord(p.User, result, c) +} + +func checkUserRecord(x, y []privileges.UserRecord, c *C) { + c.Assert(len(x), Equals, len(y)) + for i := 0; i < len(x); i++ { + c.Assert(x[i].User, Equals, y[i].User) + c.Assert(x[i].Host, Equals, y[i].Host) + } +} From 4883b7b3a27d40f0df7d565dab51e86265c1be34 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 31 Jul 2018 22:18:18 +0800 Subject: [PATCH 2/6] use `%` instead of "%" to get better visualization in github --- config/config.toml.example | 2 +- privilege/privileges/cache.go | 10 +++++----- privilege/privileges/cache_test.go | 12 ++++++------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/config/config.toml.example b/config/config.toml.example index f63397025e868..3cde9082246cf 100644 --- a/config/config.toml.example +++ b/config/config.toml.example @@ -45,7 +45,7 @@ enable-streaming = false lower-case-table-names = 2 # Make "kill query" behavior compatible with MySQL. It's not recommend to -# turn on this option when TiDB server is behand a proxy. +# turn on this option when TiDB server is behind a proxy. compatible-kill-query = false [log] diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go index 2dd8f02ba1949..6d4679e52a247 100644 --- a/privilege/privileges/cache.go +++ b/privilege/privileges/cache.go @@ -193,12 +193,12 @@ func (s sortedUserRecord) Less(i, j int) bool { return x.User < y.User } -// compareHost compares two host string using some special rule, return value 1, 0, -1 means > = <. +// compareHost compares two host string using some special rules, return value 1, 0, -1 means > = <. func compareHost(x, y string) int { // The more-specific, the smaller it is. // The pattern '%' means “any host” and is least specific. - if y == "%" { - if x == "%" { + if y == `%` { + if x == `%` { return 0 } return -1 @@ -212,8 +212,8 @@ func compareHost(x, y string) int { return -1 } - if strings.HasSuffix(y, "%") { - if !strings.HasSuffix(x, "%") { + if strings.HasSuffix(y, `%`) { + if !strings.HasSuffix(x, `%`) { return -1 } return 0 diff --git a/privilege/privileges/cache_test.go b/privilege/privileges/cache_test.go index a6c844d8672a5..88514240c1f30 100644 --- a/privilege/privileges/cache_test.go +++ b/privilege/privileges/cache_test.go @@ -265,8 +265,8 @@ func (s *testCacheSuite) TestAbnormalMySQLTable(c *C) { func (s *testCacheSuite) TestSortUserTable(c *C) { var p privileges.MySQLPrivilege p.User = []privileges.UserRecord{ - {Host: "%", User: "root"}, - {Host: "%", User: "jeffrey"}, + {Host: `%`, User: "root"}, + {Host: `%`, User: "jeffrey"}, {Host: "localhost", User: "root"}, {Host: "localhost", User: ""}, } @@ -274,19 +274,19 @@ func (s *testCacheSuite) TestSortUserTable(c *C) { result := []privileges.UserRecord{ {Host: "localhost", User: "root"}, {Host: "localhost", User: ""}, - {Host: "%", User: "jeffrey"}, - {Host: "%", User: "root"}, + {Host: `%`, User: "jeffrey"}, + {Host: `%`, User: "root"}, } checkUserRecord(p.User, result, c) p.User = []privileges.UserRecord{ - {Host: "%", User: "jeffrey"}, + {Host: `%`, User: "jeffrey"}, {Host: "h1.example.net", User: ""}, } p.SortUserTable() result = []privileges.UserRecord{ {Host: "h1.example.net", User: ""}, - {Host: "%", User: "jeffrey"}, + {Host: `%`, User: "jeffrey"}, } checkUserRecord(p.User, result, c) } From 5c7a96e9d19022bba66be666c520d90defb52689 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Wed, 1 Aug 2018 14:53:00 +0800 Subject: [PATCH 3/6] address comment --- privilege/privileges/cache.go | 16 ++++++++++++++-- privilege/privileges/cache_test.go | 11 +++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go index 6d4679e52a247..c1aa05e59137d 100644 --- a/privilege/privileges/cache.go +++ b/privilege/privileges/cache.go @@ -212,9 +212,21 @@ func compareHost(x, y string) int { return -1 } - if strings.HasSuffix(y, `%`) { - if !strings.HasSuffix(x, `%`) { + // One of them end with `%`. + xEnd := strings.HasSuffix(x, `%`) + yEnd := strings.HasSuffix(y, `%`) + if xEnd || yEnd { + switch { + case !xEnd && yEnd: return -1 + case xEnd && !yEnd: + return 1 + case xEnd && yEnd: + // 192.168.199.% smaller than 192.168.% + // A not very accurate comparison, compare them by length. + if len(x) > len(y) { + return -1 + } } return 0 } diff --git a/privilege/privileges/cache_test.go b/privilege/privileges/cache_test.go index 88514240c1f30..c834cd54c7050 100644 --- a/privilege/privileges/cache_test.go +++ b/privilege/privileges/cache_test.go @@ -289,6 +289,17 @@ func (s *testCacheSuite) TestSortUserTable(c *C) { {Host: `%`, User: "jeffrey"}, } checkUserRecord(p.User, result, c) + + p.User = []privileges.UserRecord{ + {Host: "192.168.%", User: "xxx"}, + {Host: `192.168.199.%`, User: "xxx"}, + } + p.SortUserTable() + result = []privileges.UserRecord{ + {Host: `192.168.199.%`, User: "xxx"}, + {Host: "192.168.%", User: "xxx"}, + } + checkUserRecord(p.User, result, c) } func checkUserRecord(x, y []privileges.UserRecord, c *C) { From a85f38624f6186ee8dbc31e0c76e7c22e41812b5 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Wed, 1 Aug 2018 16:11:46 +0800 Subject: [PATCH 4/6] address comment --- privilege/privileges/cache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go index c1aa05e59137d..608582bea0a46 100644 --- a/privilege/privileges/cache.go +++ b/privilege/privileges/cache.go @@ -194,6 +194,7 @@ func (s sortedUserRecord) Less(i, j int) bool { } // compareHost compares two host string using some special rules, return value 1, 0, -1 means > = <. +// TODO: Check how MySQL do it exactly, instead of guess its rules. func compareHost(x, y string) int { // The more-specific, the smaller it is. // The pattern '%' means “any host” and is least specific. From 851f8c847c48653c4b7f979374c886784905ac82 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Wed, 1 Aug 2018 16:13:31 +0800 Subject: [PATCH 5/6] address comment --- privilege/privileges/cache_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/privilege/privileges/cache_test.go b/privilege/privileges/cache_test.go index c834cd54c7050..636ce84352e3f 100644 --- a/privilege/privileges/cache_test.go +++ b/privilege/privileges/cache_test.go @@ -291,13 +291,13 @@ func (s *testCacheSuite) TestSortUserTable(c *C) { checkUserRecord(p.User, result, c) p.User = []privileges.UserRecord{ - {Host: "192.168.%", User: "xxx"}, + {Host: `192.168.%`, User: "xxx"}, {Host: `192.168.199.%`, User: "xxx"}, } p.SortUserTable() result = []privileges.UserRecord{ {Host: `192.168.199.%`, User: "xxx"}, - {Host: "192.168.%", User: "xxx"}, + {Host: `192.168.%`, User: "xxx"}, } checkUserRecord(p.User, result, c) } From d9aa8f4c195dc0387fc8518ce01d8a9a83c45f83 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Thu, 2 Aug 2018 19:34:02 +0800 Subject: [PATCH 6/6] address comment --- privilege/privileges/cache.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go index 608582bea0a46..3ff9e6b0c28bb 100644 --- a/privilege/privileges/cache.go +++ b/privilege/privileges/cache.go @@ -173,12 +173,12 @@ func (p *MySQLPrivilege) LoadUserTable(ctx sessionctx.Context) error { type sortedUserRecord []UserRecord func (s sortedUserRecord) Len() int { - return len([]UserRecord(s)) + return len(s) } func (s sortedUserRecord) Less(i, j int) bool { - x := ([]UserRecord(s))[i] - y := ([]UserRecord(s))[j] + x := s[i] + y := s[j] // Compare two item by user's host first. c1 := compareHost(x.Host, y.Host) @@ -243,8 +243,7 @@ func compareHost(x, y string) int { } func (s sortedUserRecord) Swap(i, j int) { - s1 := []UserRecord(s) - s1[i], s1[j] = s1[j], s1[i] + s[i], s[j] = s[j], s[i] } // SortUserTable sorts p.User in the MySQLPrivilege struct.