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.

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

Jump to bottom

[0.11.0] URL parse for mysql connection string fails with special characters in password #870

Closed
burdandrei opened this issue Mar 17, 2016 · 7 comments · Fixed by #1891
Closed

[0.11.0] URL parse for mysql connection string fails with special characters in password #870

burdandrei opened this issue Mar 17, 2016 · 7 comments · Fixed by #1891
Labels
bug unexpected problem or unintended behavior
Milestone
1.1.0

Comments

@burdandrei
Copy link
Contributor

I got "%j+" sequence in my mysql password, and now because url.Parse it fails:
https://github.com/influxdata/telegraf/blob/master/plugins/inputs/mysql/mysql.go#L225

Per original Package documentation special characters supported in password and should not be escaped:
https://github.com/go-sql-driver/mysql/blob/master/README.md#password

There is ParseDSN method in package:
https://github.com/go-sql-driver/mysql/blob/master/dsn.go#L245
@sparrc sparrc added the bug unexpected problem or unintended behavior label Mar 17, 2016
@lnkisi
Copy link

lnkisi commented Sep 29, 2016

Same issue.

@sparrc sparrc added this to the 1.1.0 milestone Sep 29, 2016
@maksadbek
Copy link
Contributor

@burdandrei @lnkisi Thanks for reporting this bug, I will fix it soon.

@maksadbek
Copy link
Contributor

@burdandrei @lnkisi
Can you provide sample DSN that reproduces this bug ? A password with %j+ sequence works fine for me.

@burdandrei
Copy link
Contributor Author

@maksadbek well this is strange,
I just tried this servers = ['ttt:1%j+2@tcp(production.db.com:3306)/'] and this worked for me in 1.0
@lnkisi can you please share your DSN?

@sparrc
Copy link
Contributor

sparrc commented Oct 3, 2016

see also #1842

@sparrc sparrc mentioned this issue Oct 6, 2016
Closed
sparrc added a commit that referenced this issue Oct 12, 2016
@sparrc
Use mysql.ParseDSN func instead of url.Parse
f9ea69f 
closes #870
closes #1842
@sparrc sparrc mentioned this issue Oct 12, 2016
Merged
3 tasks
sparrc added a commit that referenced this issue Oct 12, 2016
@sparrc
Use mysql.ParseDSN func instead of url.Parse
7026f54 
The MySQL DB driver has it's own DSN parsing function. Previously we
were using the url.Parse function, but this causes problems because a
valid MySQL DSN can be an invalid http URL, namely when using some
special characters in the password.

This change uses the MySQL DB driver's builtin ParseDSN function and
applies a timeout parameter natively via that.

Another benefit of this change is that we fail earlier if given an
invalid MySQL DSN.

closes #870
closes #1842
sparrc added a commit that referenced this issue Oct 12, 2016
@sparrc
Use mysql.ParseDSN func instead of url.Parse
245ba5a 
The MySQL DB driver has it's own DSN parsing function. Previously we
were using the url.Parse function, but this causes problems because a
valid MySQL DSN can be an invalid http URL, namely when using some
special characters in the password.

This change uses the MySQL DB driver's builtin ParseDSN function and
applies a timeout parameter natively via that.

Another benefit of this change is that we fail earlier if given an
invalid MySQL DSN.

closes #870
closes #1842
sparrc added a commit that referenced this issue Oct 12, 2016
@sparrc
Use mysql.ParseDSN func instead of url.Parse
a65447d 
The MySQL DB driver has it's own DSN parsing function. Previously we
were using the url.Parse function, but this causes problems because a
valid MySQL DSN can be an invalid http URL, namely when using some
special characters in the password.

This change uses the MySQL DB driver's builtin ParseDSN function and
applies a timeout parameter natively via that.

Another benefit of this change is that we fail earlier if given an
invalid MySQL DSN.

closes #870
closes #1842
@adityask2
Copy link

Has this issue been fixed? I'm using v1.2.1. Seems like this issue still exists

@danielnelson
Copy link
Contributor

@adityask2 The fix above should be in 1.2.1, can you open a new issue and include a sample connection string that replicates your issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug unexpected problem or unintended behavior
Projects
None yet
Milestone
1.1.0
Development

Successfully merging a pull request may close this issue.

Use mysql.ParseDSN func instead of url.Parse influxdata/telegraf
6 participants
@danielnelson @maksadbek @burdandrei @lnkisi @sparrc @adityask2