address PR feedback

client/auth.go

@@ -281,12 +281,12 @@ func (c *Conn) writeAuthHandshake() error {
 
 // the MySQL protocol calls for the collation id to be sent as 1, where only the
 // lower 8 bits are used in this field. But wireshark shows that the first by of
-// the 23 bytes of filler is used to send the upper 8 bits of the collation id.
+// 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 collation.ID > 255 {
-data[13] = byte(collation.ID >> 8)
-}
+// if the collation ID is <= 255 the middle 8 bits are 0s so this is the equivalent of
+// padding the filler with a 0.
+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
@@ -308,13 +308,12 @@ func (c *Conn) writeAuthHandshake() error {
 }
 
 // Filler [23 bytes] (all 0x00)
-pos := 13
-if collation.ID > 255 {
-// skip setting the first byte of the filler to 0x00 since it is used to
-// send the upper 8 bits of the collation id
-pos++
-}
-for ; pos < 13+23; pos++ {
+// the filler starts at position 13, but the first byte of the filler
+// maybe have been set by the collation id earlier. So we only position 13
+// will be either 0x00 or the right middle 8 bits of the collation id. Therefore
+// here we start at position 14 and fill the remaining 22 bytes with 0x00.
+pos := 14
+for ; pos < 14+22; pos++ {
 data[pos] = 0
 }
 

client/auth_test.go

@@ -39,13 +39,15 @@ func TestConnGenAttributes(t *testing.T) {
 }
 
 func TestConnCollation(t *testing.T) {
-collations := []string{"big5_chinese_ci",
-"utf8_general_ci",
-"utf8mb4_0900_ai_ci",
-"utf8mb4_de_pb_0900_ai_ci",
-"utf8mb4_ja_0900_as_cs",
-"utf8mb4_0900_bin",
-"utf8mb4_zh_pinyin_tidb_as_cs"}
+collations := []string{
+//"big5_chinese_ci",
+//"utf8_general_ci",
+//"utf8mb4_0900_ai_ci",
+//"utf8mb4_de_pb_0900_ai_ci",
+//"utf8mb4_ja_0900_as_cs",
+//"utf8mb4_0900_bin",
+"utf8mb4_zh_pinyin_tidb_as_cs",
+}
 
 // test all supported collations by calling writeAuthHandshake() and reading the bytes
 // sent to the server to ensure the collation id is set correctly
@@ -63,19 +65,17 @@ 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])
-// sanity check: validate the 23 bytes of filler with value 0x00 are set correctly
-for i := 13; i < 13+23; i++ {
-require.Equal(t, byte(0x00), handShakeResponse[i])
-}
+// 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
 require.Equal(t, byte(collation.ID&0xff), handShakeResponse[12])
 require.Equal(t, byte(collation.ID>>8), handShakeResponse[13])
+}
 
-// sanity check: validate the 22 bytes of filler with value 0x00 are set correctly
-for i := 14; i < 14+22; i++ {
-require.Equal(t, byte(0x00), handShakeResponse[i])
-}
+// sanity check: validate the 22 bytes of filler with value 0x00 are set correctly
+for i := 14; i < 14+22; i++ {
+require.Equal(t, byte(0x00), handShakeResponse[i])
 }
 
 // and finally the username