-
-
Notifications
You must be signed in to change notification settings - Fork 625
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
Adds decodeURIComponent in ConnectionConfig#parseUrl #1621
Conversation
This commit adds decodeURIComponent to the ConnectionConfig#parseUrl, for options.user and options.password.
@Paultje52 I'm really looking forward to seeing this PR merged, as I cannot use I have tested with this list of available characters in MySQL strong passwords ( For this list const password = "~!@$^&*()_-+={}[]<>,.;':|"
const connectionString = `mysql://username:${password}@127.0.0.1/database`
const parsedUrl = new URL(connectionString)
const parsedPassword = decodeURIComponent(parsedUrl.password)
console.log(parsedPassword === password) // true But if the password is including one of const password = "~!@$^&*()_-+={}[]<>,.;':|" + "#?/"
const connectionString = `mysql://username:${password}@127.0.0.1/database`
const parsedUrl = new URL(connectionString) // Invalid URL Also if the password is including const password = "~!@$^&*()_-+={}[]<>,.;':|" + "%"
const connectionString = `mysql://username:${password}@127.0.0.1/database`
const parsedUrl = new URL(connectionString)
const parsedPassword = decodeURIComponent(parsedUrl.password) // URI malformed However we can fix the - const parsedUrl = new URL(url);
+ const parsedUrl = new URL(encodeURI(url));
const options = {
host: parsedUrl.hostname,
port: parsedUrl.port,
database: parsedUrl.pathname.slice(1),
user: decodeURIComponent(parsedUrl.username),
password: decodeURIComponent(parsedUrl.password)
}; This will make const password = "~!@$^&*()_-+={}[]<>,.;':|" + "%"
const connectionString = `mysql://username:${password}@127.0.0.1/database`
const parsedUrl = new URL(encodeURI(connectionString))
const parsedPassword = decodeURIComponent(parsedUrl.password)
console.log(parsedPassword === password) // true @sidorares Despite not covering every edge case (
|
Thanks @Paultje52 ! Would you be able to add a unit test for this? |
@kykungz and @sidorares I changed the parser to support all special characters. Let me know what you think! If you approve this, I can start working on the unit test :-) |
|
@sidorares @Paultje52 After digging around with MySQL's URI-Like Connection Strings, I found that both solution (f08b3be and ed0cb99) will never cover every use-case. For example, using manual parsing will cover more use-cases, but it will open up another limitation of:
while MySQL natively supported Although there will be only a few people who hit this wall, it's not great that a widely-used lib like this cannot match what MySQL can support. It is explicitly stated in MySQL docs that you must encode all special characters before using it:
So you cannot use mysqlsh --uri "mysql://username:p@$$w&rd@127.0.0.1:3306/database"
# ❌ Error - Invalid URI You must mysqlsh --uri "mysql://username:p%40%24%24w%26rd@127.0.0.1:3306/database"
# ✅ Connected It wouldn't make sense if you can connect to MySQL through node-mysql2 with So I think we should re-consider this and make this lib consistent with MySQL's standard, to eliminate all confusions about the lib vs actual MySQL. Instead of auto parsing connection string, we should be thinking of how to support encoded connection string. We can simply revert back to f08b3be, and maybe add more const options = {
- host: parsedUrl.hostname,
+ host: decodeURIComponent(parsedUrl.hostname),
port: parsedUrl.port,
- database: parsedUrl.pathname.slice(1),
+ database: decodeURIComponent(parsedUrl.pathname.slice(1)),
user: decodeURIComponent(parsedUrl.username),
password: decodeURIComponent(parsedUrl.password)
}; After that we can add more documentation on how to use URI-Like Connection Strings (every special character must be encoded) by referring to this document. What do you think? |
@kykungz I'm happy with the proposed solution ( throwing in few while we are at this, might worth checking/adding support for socket path in uri connection:
I think at this stage its more important to keep compatibility with mysqljs/mysql rather than mysql cli or other tools but ideally we should be following the spec in the https://dev.mysql.com/doc/refman/8.0/en/connecting-using-uri-or-key-value-pairs.html |
Any progress on this one? Running into this right now, unable to connect to my database at all due to crazy passsword... |
I ended up hand-patching connection_config.js and it worked perfectly, FWIW. :) |
Currently, you cannot use special characters like
@
in usernames or passwords. This is because those characters are getting uri encoded by the URL nodejs package.This PR adds
decodeURIComponent()
toConnectionConfig#parseUrl
, foroptions.user
andoptions.password
. This way, users can use special characters like@
in usernames and passwords!This also resolves #1384.
It also makes sure mysql2 can be used in nodejs apps hosted by pterodactyl with databases. When creating a database in pterodactyl, it generates a password automatically. That password also contains special characters, which make them useless when using mysql2.