-
Notifications
You must be signed in to change notification settings - Fork 38
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.
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
Support for latest Connector/J 8.0.22 #14
base: main
Are you sure you want to change the base?
Conversation
My apologies. I didn't |
Thank you for digging into this! I'll give it a read now \o/ |
@@ -238,15 +238,22 @@ impl<B: MysqlShim<W>, R: Read, W: Write> MysqlIntermediary<B, R, W> { | |||
fn init(&mut self) -> Result<(), B::Error> { | |||
self.writer.write_all(&[10])?; // protocol 10 | |||
|
|||
let mut capabilities = CapabilityFlags::empty(); | |||
capabilities.insert(CapabilityFlags::CLIENT_PROTOCOL_41); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the handshake change the only difference with the 4.1 protocol?
@@ -238,15 +238,22 @@ impl<B: MysqlShim<W>, R: Read, W: Write> MysqlIntermediary<B, R, W> { | |||
fn init(&mut self) -> Result<(), B::Error> { | |||
self.writer.write_all(&[10])?; // protocol 10 | |||
|
|||
let mut capabilities = CapabilityFlags::empty(); | |||
capabilities.insert(CapabilityFlags::CLIENT_PROTOCOL_41); | |||
capabilities.insert(CapabilityFlags::CLIENT_RESERVED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does setting this do? Aren't generally reserved flags supposed to be 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It came from default capabilities flags. Here you set it.
Line 246 in 4f301c9
self.writer.write_all(&[0x00, 0x42])?; // just 4.1 proto |
It should be
0x02
if you only meant CLIENT_PROTOCOL_41
.
let mut capabilities = CapabilityFlags::empty(); | ||
capabilities.insert(CapabilityFlags::CLIENT_PROTOCOL_41); | ||
capabilities.insert(CapabilityFlags::CLIENT_RESERVED); | ||
capabilities.insert(CapabilityFlags::CLIENT_SECURE_CONNECTION); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually support secure connections? I did not think so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't. But those two are the minimal sets of flags that fixed the connection phase issue with the latest Connector/J. But I have to fix the client authentication problem.
capabilities.insert(CapabilityFlags::CLIENT_PROTOCOL_41); | ||
capabilities.insert(CapabilityFlags::CLIENT_RESERVED); | ||
capabilities.insert(CapabilityFlags::CLIENT_SECURE_CONNECTION); | ||
capabilities.insert(CapabilityFlags::CLIENT_PLUGIN_AUTH); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What makes it so that we support plugin auths?
This generally looks great subject to the tests passing and the comments I left above! |
This should fix mit-pdos/noria-mysql#15