Skip to content

client: make CLIENT_QUERY_ATTRIBUTES opt-out to restore proxy compatibility #1041

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

Merged
merged 4 commits into from
Jun 8, 2025
Merged

client: make CLIENT_QUERY_ATTRIBUTES opt-out to restore proxy compatibility #1041

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5374478
feat: Update client capability flags
masahide May 31, 2025
08ffcfe
feat: Update client capability flags
masahide Jun 7, 2025
abad2bf
fix comment
masahide Jun 7, 2025
96aa710
Merge branch 'master' into add-dcaps
masahide Jun 7, 2025
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
13 changes: 11 additions & 2 deletions client/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,15 @@ func (c *Conn) writeAuthHandshake() error {
capability := mysql.CLIENT_PROTOCOL_41 | mysql.CLIENT_SECURE_CONNECTION |
mysql.CLIENT_LONG_PASSWORD | mysql.CLIENT_TRANSACTIONS | mysql.CLIENT_PLUGIN_AUTH
// Adjust client capability flags based on server support
capability |= c.capability & mysql.CLIENT_LONG_FLAG
capability |= c.capability & mysql.CLIENT_QUERY_ATTRIBUTES
Copy link
Collaborator

@lance6716 lance6716 Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think we don't need to specially process CLIENT_QUERY_ATTRIBUTES. There are 3 states of capability:

  1. developer calls SetCapability, which means the ccaps bit is 1 and the dcaps bit is 0
  2. developer calls UnsetCapability, which means ccaps bit is 0 and the dcaps bit is 1
  3. No calls, which means both bits are 0

So here we turn on the bits by ccaps at around line 219, and have another round to turn off bits by dcaps. I suggest

capability |= c.ccaps&mysql.CLIENT_FOUND_ROWS | ...
capability &= c.dcaps... // ⬅️ here we add CLIENT_QUERY_ATTRIBUTES and other flags from above line to turn off

// ---- Inherit capabilities that the server has and the user has NOT explicitly denied ----
inherit := c.capability & ^c.dcaps // Server-side capabilities minus explicit denies
Copy link
Collaborator

@dveeden dveeden Jun 3, 2025

Choose a reason for hiding this comment

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

maybe name this inheritedCaps or inherited?

capability |= inherit & mysql.CLIENT_LONG_FLAG // Existing
// ---- Handling of CLIENT_QUERY_ATTRIBUTES ----
if c.ccaps&mysql.CLIENT_QUERY_ATTRIBUTES != 0 { // Explicitly ON
capability |= mysql.CLIENT_QUERY_ATTRIBUTES
} else if inherit&mysql.CLIENT_QUERY_ATTRIBUTES != 0 { // Server has it and no denial
capability |= mysql.CLIENT_QUERY_ATTRIBUTES
}
// Adjust client capability flags on specific client requests
// Only flags that would make any sense setting and aren't handled elsewhere
// in the library are supported here
Expand Down Expand Up @@ -361,5 +368,7 @@ func (c *Conn) writeAuthHandshake() error {
data[pos] = 0x03
}

c.capability = capability // update capability to the one we sent

return c.WritePacket(data)
}
5 changes: 4 additions & 1 deletion client/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type Conn struct {
capability uint32
// client-set capabilities only
ccaps uint32
dcaps uint32 // disabled capabilities
Copy link
Collaborator

@lance6716 lance6716 Jun 3, 2025

Choose a reason for hiding this comment

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

I see you followed the naming style of ccaps. Just a weak comment: capability, ccaps and dcaps are hard to understand at their usage functions. Maybe rename them to serverCaps, clientExplicitOnCaps, clientExplicitOffCaps or something.


attributes map[string]string

Expand Down Expand Up @@ -236,12 +237,14 @@ func (c *Conn) Ping() error {

// SetCapability enables the use of a specific capability
func (c *Conn) SetCapability(cap uint32) {
c.dcaps &^= cap
c.ccaps |= cap
}

// UnsetCapability disables the use of a specific capability
func (c *Conn) UnsetCapability(cap uint32) {
c.ccaps &= ^cap
c.ccaps &^= cap
c.dcaps |= cap
}

// HasCapability returns true if the connection has the specific capability
Expand Down
Loading