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

collation in auth handshake are 1-byte #865

Merged
merged 3 commits into from
May 7, 2024
Merged

collation in auth handshake are 1-byte #865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2a3d211
collation in auth handshake are 1-byte
dvilaverde May 6, 2024
b03ff0c
updating SET NAMES statement for collation
dvilaverde May 6, 2024
bd49903
fixing golangci-lint config
dvilaverde May 6, 2024
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
3 changes: 0 additions & 3 deletions .golangci.yml
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

These don't look related to the rest of the PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
WARN [runner] The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
WARN [linters_context] structcheck is disabled because of generics. You can track the evolution of the generics support by following the https://github.com/golangci/golangci-lint/issues/2649.

Ok. So they are deprecated. Indeed good to remove them. Thanks.

Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@ linters:
disable-all: true
enable:
# All code is ready for:
- deadcode
- errcheck
- staticcheck
- structcheck
- typecheck
- unused
- varcheck
- misspell
- nolintlint
- goimports
Expand Down
12 changes: 2 additions & 10 deletions client/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,6 @@ func (c *Conn) writeAuthHandshake() error {
// the 23 bytes of filler is used to send the right middle 8 bits of the collation id.
// see https://github.com/mysql/mysql-server/pull/541
data[12] = byte(collation.ID & 0xff)
// if the collation ID is <= 255 the middle 8 bits are 0s so this is the equivalent of
// padding the filler with a 0. If ID is > 255 then the first byte of filler will contain
// the right middle 8 bits of the collation ID.
data[13] = byte((collation.ID & 0xff00) >> 8)

// SSL Connection Request Packet
// http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::SSLRequest
Expand All @@ -309,12 +305,8 @@ func (c *Conn) writeAuthHandshake() error {
}

// Filler [23 bytes] (all 0x00)
// the filler starts at position 13, but the first byte of the filler
// has been set with the collation id earlier, so position 13 at this point
// will be either 0x00, or the right middle 8 bits of the collation id.
// Therefore, we start at position 14 and fill the remaining 22 bytes with 0x00.
pos := 14
for ; pos < 14+22; pos++ {
pos := 13
for ; pos < 13+23; pos++ {
data[pos] = 0
}

Expand Down
10 changes: 5 additions & 5 deletions client/auth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ func TestConnCollation(t *testing.T) {
// if the collation ID is <= 255 the collation ID is stored in the 12th byte
if collation.ID <= 255 {
require.Equal(t, byte(collation.ID), handShakeResponse[12])
// the 13th byte should always be 0x00
require.Equal(t, byte(0x00), handShakeResponse[13])
} else {
// if the collation ID is > 255 the collation ID is stored in the 12th and 13th bytes
// if the collation ID is > 255 the collation ID should just be the lower-8 bits
require.Equal(t, byte(collation.ID&0xff), handShakeResponse[12])
require.Equal(t, byte(collation.ID>>8), handShakeResponse[13])
}

// the 13th byte should always be 0x00
require.Equal(t, byte(0x00), handShakeResponse[13])

// sanity check: validate the 22 bytes of filler with value 0x00 are set correctly
for i := 14; i < 14+22; i++ {
for i := 13; i < 13+23; i++ {
require.Equal(t, byte(0x00), handShakeResponse[i])
}

Expand Down
16 changes: 16 additions & 0 deletions client/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"time"

"github.com/pingcap/errors"
"github.com/pingcap/tidb/pkg/parser/charset"

. "github.com/go-mysql-org/go-mysql/mysql"
"github.com/go-mysql-org/go-mysql/packet"
Expand Down Expand Up @@ -133,6 +134,21 @@ func ConnectWithDialer(ctx context.Context, network string, addr string, user st
c.Conn.Compression = MYSQL_COMPRESS_ZSTD
}

// if a collation was set with a ID of > 255, then we need to call SET NAMES ...
// since the auth handshake response only support collations with 1-byte ids
if len(c.collation) != 0 {
collation, err := charset.GetCollationByName(c.collation)
if err != nil {
return nil, errors.Trace(fmt.Errorf("invalid collation name %s", c.collation))
}

if collation.ID > 255 {
if _, err := c.exec(fmt.Sprintf("SET NAMES %s COLLATE %s", c.charset, c.collation)); err != nil {
return nil, errors.Trace(err)
}
}
}

return c, nil
}

Expand Down
Loading