Skip to content

Commit

Permalink
server: support for client multi-statement option (pingcap#19459) (pi…
Browse files Browse the repository at this point in the history
…ngcap#20408)

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
  • Loading branch information
ti-srebot authored Nov 6, 2020
1 parent 19d8e37 commit 0b0c41b
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 1 deletion.
10 changes: 10 additions & 0 deletions server/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,16 @@ func (cc *clientConn) handleQuery(ctx context.Context, sql string) (err error) {
if len(rss) == 1 {
err = cc.writeResultset(ctx, rss[0], false, 0, 0)
} else {
// The client gets to choose if it allows multi-statements, and
// probably defaults OFF. This helps prevent against SQL injection attacks
// by early terminating the first statement, and then running an entirely
// new statement.

capabilities := cc.ctx.GetSessionVars().ClientCapability
if capabilities&mysql.ClientMultiStatements < 1 {
return errMultiStatementDisabled
}

err = cc.writeMultiResultset(ctx, rss, false)
}
} else {
Expand Down
1 change: 1 addition & 0 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ var (
errAccessDenied = terror.ClassServer.New(errno.ErrAccessDenied, errno.MySQLErrName[errno.ErrAccessDenied])
errConCount = terror.ClassServer.New(errno.ErrConCount, errno.MySQLErrName[errno.ErrConCount])
errSecureTransportRequired = terror.ClassServer.New(errno.ErrSecureTransportRequired, errno.MySQLErrName[errno.ErrSecureTransportRequired])
errMultiStatementDisabled = terror.ClassServer.New(errno.ErrUnknown, "client has multi-statement capability disabled") // MySQL returns a parse error
)

// DefaultCapability is the capability of the server when it is created using the default configuration.
Expand Down
16 changes: 15 additions & 1 deletion server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,8 +1082,22 @@ func (cli *testServerClient) runTestStatusAPI(c *C) {
c.Assert(data.GitHash, Equals, versioninfo.TiDBGitHash)
}

// The golang sql driver (and most drivers) should have multi-statement
// disabled by default for security reasons. Lets ensure that the behavior
// is correct.

func (cli *testServerClient) runFailedTestMultiStatements(c *C) {
cli.runTestsOnNewDB(c, nil, "FailedMultiStatements", func(dbt *DBTest) {
_, err := dbt.db.Exec("SELECT 1; SELECT 1; SELECT 2; SELECT 3;")
c.Assert(err.Error(), Equals, "Error 1105: client has multi-statement capability disabled")
})
}

func (cli *testServerClient) runTestMultiStatements(c *C) {
cli.runTestsOnNewDB(c, nil, "MultiStatements", func(dbt *DBTest) {

cli.runTestsOnNewDB(c, func(config *mysql.Config) {
config.Params = map[string]string{"multiStatements": "true"}
}, "MultiStatements", func(dbt *DBTest) {
// Create Table
dbt.mustExec("CREATE TABLE `test` (`id` int(11) NOT NULL, `value` int(11) NOT NULL) ")

Expand Down
1 change: 1 addition & 0 deletions server/tidb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ func newTLSHttpClient(c *C, caFile, certFile, keyFile string) *http.Client {

func (ts *tidbTestSuite) TestMultiStatements(c *C) {
c.Parallel()
ts.runFailedTestMultiStatements(c)
ts.runTestMultiStatements(c)
}

Expand Down

0 comments on commit 0b0c41b

Please sign in to comment.