Skip to content

parse dsn fix #606

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

Closed
wants to merge 2 commits into from
Closed

parse dsn fix #606

wants to merge 2 commits into from

Conversation

ans-
Copy link

@ans- ans- commented May 30, 2017

#591

Description

I encounter the case ':' in username, ParseDSN can't get the expect result.

my scripts parse "[username[:password]" part reversely, and compatible with old code.

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

@julienschmidt
Copy link
Member

This would break support for passwords containing colons

@coveralls
Copy link

coveralls commented May 30, 2017

Coverage Status

Coverage increased (+0.03%) to 74.918% when pulling 6c44b43 on ans-:parse-dsn-fix into 44fa292 on go-sql-driver:master.

@julienschmidt
Copy link
Member

Your test case is user:name:password. In the current implementation we decided that everything left of the first colon is the password. This PR would change the behavior to consider everything of the rightmost colon to be the username, breaking every setup where the password contains a colon (e.g. name:password, like in the test case).

Without additional escaping, it is impossible to support colons in both the username and the password at the same time. Even introducing escaping is likely to break configurations. See the discussion in #591.
That's why we decided to no change the behavior for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants