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.

Sign up for GitHub

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

Jump to bottom

sessionctx, variable: add skip name resolve #25464

Merged
merged 4 commits into from
Jun 22, 2021
Merged

sessionctx, variable: add skip name resolve #25464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d7045ab
session, variable: fix linter issues
morgo Jun 15, 2021
f52466d
sessionctx, variable: add skip name resolve
morgo Jun 15, 2021
a32bcfe
Merge branch 'master' into add-skip-name-resolve
morgo Jun 16, 2021
e08a0d6
Merge branch 'master' into add-skip-name-resolve
ti-chi-bot Jun 22, 2021
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
20 changes: 15 additions & 5 deletions session/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2160,7 +2160,7 @@ func (s *session) Auth(user *auth.UserIdentity, authentication []byte, salt []by
}

// Check Hostname.
for _, addr := range getHostByIP(user.Hostname) {
for _, addr := range s.getHostByIP(user.Hostname) {
u, h, success := pm.ConnectionVerification(user.Username, addr, authentication, salt, s.sessionVars.TLSConnectionState)
if success {
s.sessionVars.User = &auth.UserIdentity{
Expand Down Expand Up @@ -2192,7 +2192,7 @@ func (s *session) AuthWithoutVerification(user *auth.UserIdentity) bool {
}

// Check Hostname.
for _, addr := range getHostByIP(user.Hostname) {
for _, addr := range s.getHostByIP(user.Hostname) {
u, h, success := pm.GetAuthWithoutVerification(user.Username, addr)
if success {
s.sessionVars.User = &auth.UserIdentity{
Expand All @@ -2208,14 +2208,24 @@ func (s *session) AuthWithoutVerification(user *auth.UserIdentity) bool {
return false
}

func getHostByIP(ip string) []string {
func (s *session) getHostByIP(ip string) []string {
if ip == "127.0.0.1" {
return []string{variable.DefHostname}
}
skipNameResolve, err := s.GetSessionVars().GlobalVarsAccessor.GetGlobalSysVar(variable.SkipNameResolve)
if err == nil && variable.TiDBOptOn(skipNameResolve) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you intended to continue even when err != nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The option defaults to OFF (and always will). It's only if can be successfully read as ON that we take the short-path and return the IP address. See: https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_skip_name_resolve

return []string{ip} // user wants to skip name resolution
}
addrs, err := net.LookupAddr(ip)
if err != nil {
// The error is ignorable.
// The empty line here makes the golint tool (which complains err is not checked) happy.
// These messages can be noisy.
// See: https://github.com/pingcap/tidb/pull/13989
logutil.BgLogger().Debug(
"net.LookupAddr returned an error during auth check",
zap.String("ip", ip),
zap.Error(err),
)
return []string{ip}
}
return addrs
}
Expand Down
1 change: 0 additions & 1 deletion sessionctx/variable/noop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ var noopSysVars = []*SysVar{
{Scope: ScopeNone, Name: "lc_messages_dir", Value: "/usr/local/mysql-5.6.25-osx10.8-x86_64/share/"},
{Scope: ScopeGlobal, Name: "ft_boolean_syntax", Value: "+ -><()~*:\"\"&|"},
{Scope: ScopeGlobal, Name: TableDefinitionCache, Value: "2000", Type: TypeUnsigned, MinValue: 400, MaxValue: 524288, AutoConvertOutOfRange: true},
{Scope: ScopeNone, Name: SkipNameResolve, Value: Off, Type: TypeBool},
{Scope: ScopeNone, Name: "performance_schema_max_file_handles", Value: "32768"},
{Scope: ScopeSession, Name: "transaction_allow_batching", Value: ""},
{Scope: ScopeNone, Name: "performance_schema_max_statement_classes", Value: "168"},
Expand Down
1 change: 1 addition & 0 deletions sessionctx/variable/sysvar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1743,6 +1743,7 @@ var defaultSysVars = []*SysVar{
s.EnableGlobalTemporaryTable = TiDBOptOn(val)
return nil
}},
{Scope: ScopeGlobal, Name: SkipNameResolve, Value: Off, Type: TypeBool},
}

// FeedbackProbability points to the FeedbackProbability in statistics package.
Expand Down
2 changes: 2 additions & 0 deletions sessionctx/variable/sysvar_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,10 @@ func (*testSysVarSuite) TestRegistrationOfNewSysVar(c *C) {
// Boolean oN or 1 converts to canonical ON or OFF
normalizedVal, err := sysVar.Validate(vars, "oN", ScopeSession)
c.Assert(normalizedVal, Equals, "ON")
c.Assert(err, IsNil)
normalizedVal, err = sysVar.Validate(vars, "0", ScopeSession)
c.Assert(normalizedVal, Equals, "OFF")
c.Assert(err, IsNil)

err = sysVar.SetSessionFromHook(vars, "OFF") // default is on
c.Assert(err.Error(), Matches, "set should fail")
Expand Down