mysqljs / mysql

A pure node.js JavaScript Client implementing the MySQL protocol.
MIT License
18.27k stars 2.52k forks source link

MySQL 8 incompatibilities #1959

Open ruiquelhas opened 6 years ago

ruiquelhas commented 6 years ago

I work at the MySQL connectors team at Oracle and thought about running some tests using this module and the latest MySQL 8 series versions to check for incompatibilities.

There are a couple of things that need to change if there are plans to support those versions.

First of all, MySQL 8.0.4 introduces a new default authentication plugin - caching_sha2_pasword. This is a breaking change and it will affect the entire handshake process, which currently does not work for new user accounts created using this plugin or, most importantly, not using a specific plugin at all (since it's now the default option). User accounts created with the mysql_native_password plugin will still work though.

Additionally, MySQL 8.0.2 disables the local_infile system variable by default, which means that LOAD DATA LOCAL INFILE statements require the user to enable that feature explicitly (using SET GLOBAL local_infile = true). I guess this is mostly a documentation concern, however there are a couple of tests that are failing, so I thought it was also worth mentioning.

I'm working on a pull request to address these issues, but before moving on, I wanted to understand if the goal is to eventually support the MySQL 8 series or not, and what's the policy regarding non-GA versions, which is still the case for MySQL 8.0.4 (although it's already in RC, which might mean something). This last point is important because there are breaking changes between DMR releases and I wanted to understand if the patch(es) would need to factor those in.

dougwilson commented 6 years ago

Hi @ruiquelhas yea, we defiantly want to support all the versions. Here are a couple of comments inline for a few points:

Additionally, MySQL 8.0.2 disables the local_infile system variable by default, which means that LOAD DATA LOCAL INFILE statements require the user to enable that feature explicitly (using SET GLOBAL local_infile = true). I guess this is mostly a documentation concern, however there are a couple of tests that are failing, so I thought it was also worth mentioning.

Yea, to accept the changes, we need to pass on CI. This would mean that you'd want to alter our CI configuration to add MySQL 8 to the testing matrix. This MySQL 8 would be setup to enable the INFILE command, that's no problem to test it in our CI. But yea, we'd probably want to add to the top of the file a skip like some of the other tests do, that will skip the test when INFILE is not enabled, such that we'll have the INFILE tests running on MySQL 8 in our CI, but if someone runs our suite locally, those tests would skip if the user didn't enable the INFILE support.

I'm working on a pull request to address these issues, but before moving on, I wanted to understand if the goal is to eventually support the MySQL 8 series or not, and what's the policy regarding non-GA versions, which is still the case for MySQL 8.0.4 (although it's already in RC, which might mean something). This last point is important because there are breaking changes between DMR releases and I wanted to understand if the patch(es) would need to factor those in.

We defiantly want to support whatever the MySQL versions are. There is no specific policy about support for GA / non-GA. Just add it sounds fine to me. I'm not sure I understand what you're saying about DMR releases, so I don't understand how to answer that part.

ruiquelhas commented 6 years ago

Cool! A few additional points.

Yea, to accept the changes, we need to pass on CI. This would mean that you'd want to alter our CI configuration to add MySQL 8 to the testing matrix. This MySQL 8 would be setup to enable the INFILE command, that's no problem to test it in our CI.

Actually, the CI can just bootstrap the server. The specific tests can enable the feature by themselves, since it just won't have any effect on older versions. But we can discuss that after looking at the PR.

I'm not sure I understand what you're saying about DMR releases, so I don't understand how to answer that part.

Basically what I mean is that there were breaking changes between MySQL 8.0.3 and 8.0.4, and the possibility of that happening will always exist until it reaches GA. This is probably a moot point, but the reason I asked in the first place is that I know, for a fact, there will be another breaking change in the caching_sha2_password handshake process on the next MySQL version. However it's one that can be easily written off if we expose a lower-level crypto API to the user, making it a bit more customizable and allowing users to easily connect to both 8.0.4 and the next version as well. Again, this is something we can discuss after looking at the PR.

Thanks!

johannes commented 6 years ago

However it's one that can be easily written off if we expose a lower-level crypto API to the user, making it a bit more customizable

Technically the client can know the server version (the classic, contrary to the new X protocol sends the version in it's welcome packet) and decide based on <=8.0.4, however I doubt there is sense to support pre-GA stuff once GA is out.

And idea for that specific issue might be that full caching_sha256_password is only supported via SSL encrypted connections, till we are clear about the breakage on non-SSl connections and support for that is added/enabled once we have clarity, by MySQL 8 GA the latest. This would allow users of this module to test their apps early, even though they have to run with SSL which is a tiny bit slower (few rountrips more when establishing the connection) but strongly suggested for security. (MySQL 8 tries hard to generate certificates on all setups and all the ways it can be installed, so should work for all users out of the box)

Pseudo-code:

if (fast_path_fails && !tls) {
  error("caching_sha256 authentication failed on fast path, please use a secure connection or different authentication method, as full authentication is not yet supported by this module");
}
ruiquelhas commented 6 years ago

Yeah, I forgot about that possibility as well. Sounds a bit better and makes the API a bit more simple. It will only be relevant on 8.0.4 though, which might be a bit lame, but at least it becomes able to support every 8.0.x version.

hieuhlc commented 5 years ago

Hi everyone, may I ask the status of this issue and related PR? I saw the PR was locked

dougwilson commented 5 years ago

As an update on this a few days ago a new PR https://github.com/mysqljs/mysql/pull/2233 has been opened with issues addressed and tests passing. I will be reviewing this week to land as soon as possible.

aprilmintacpineda commented 5 years ago

I'm experiencing this too.

mysql --version
mysql  Ver 8.0.16 for osx10.12 on x86_64 (Homebrew)

I'm using promise-mysql@4.1.0 looking at their package.json, I could see mysql^2.17.1 in their dependencies...