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.

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

fix(BinaryLogClient): use writeBuffered exclusively & adjust packetNum for SSL switch auth #274

Merged
merged 2 commits into from
May 12, 2019

Conversation

glarwood
Copy link
Collaborator

@glarwood glarwood commented May 8, 2019

The changes introduced in #272 by @osheroff were not quite enough for me to connect to Azure MySQL. Testing against two different Azure MySQL DB's, I found that we needed to use PacketChannel#writeBuffered for each command.

Additionally, for connections using an SSL socket, the packet number needed to be doubly incremented in between the AuthenticateCommand packet and the AuthenticateNativePasswordCommand packet. This is presumably because of an implicit SSL packet being sent by the socket before the issuing of the AuthenticateNativePasswordCommand packet.

I've tested against standard MySQL databases versions (5.7) and found no problems exclusively using PacketChannel#writeBuffer both with/without SSL. I'll test it out with a few other versions and report back. @shyiko, do you anticipate any issues using PacketChannel#writeBuffer in the general case?

@glarwood glarwood requested a review from shyiko May 8, 2019 22:58
@shyiko
Copy link
Owner

shyiko commented May 12, 2019

Thanks Gabriel. This is great!
The only concern that I have is that removing

public void write(Command command, int packetNumber) throws IOException
public void write(Command command) throws IOException

from PacketChannel is a breaking change (I'll take care of it).
Merging in 🙇‍♂️

@shyiko shyiko merged commit 2e93865 into master May 12, 2019
shyiko pushed a commit that referenced this pull request May 12, 2019
@shyiko
Copy link
Owner

shyiko commented May 12, 2019

@glarwood 0.20.1 uploaded to Maven Central.

@glarwood glarwood deleted the fix-client-for-azure branch May 13, 2019 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants