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

ParseDSN doesn't support usernames containing colons #591

Open
s7v7nislands opened this issue May 11, 2017 · 9 comments
Open

ParseDSN doesn't support usernames containing colons #591

s7v7nislands opened this issue May 11, 2017 · 9 comments

Comments

@s7v7nislands
Copy link
Contributor

s7v7nislands commented May 11, 2017

Issue description

ParseDSN unspoport ":" in username

Example code

func main() {
	config := &mysql.Config{
		User:   "user:db",
		Passwd: "test",
		Net:    "tcp",
		Addr:   "127.0.0.1",
		DBName: "test",
	}

	s := config.FormatDSN()
	fmt.Printf("dsn: %s\n", s)

	config2, err := mysql.ParseDSN(s)
	if err != nil {
		log.Fatal(err)
	}
	fmt.Printf("user: %s, pass: %s\n", config2.User, config2.Passwd)

}
@csyangchen
Copy link

may I suggest use url.UserInfo to format and parse user:passwd field?

also, use url compliant dsn scheme would be great, and save a lot dsn related parsing / formating code. for example tcp://user:pass@host:port/db?param1=value1&...

@arnehormann
Copy link
Member

😟 I just checked the docs and then tried with the commandline client:

mysql> select user from mysql.user where user = '!: \\';
+------+
| user |
+------+
| !: \ |
+------+
1 row in set (0.00 sec)

mysql> CREATE USER '!: %\\';
Query OK, 0 rows affected (0.00 sec)

mysql> select user from mysql.user where user = '!: %\\';
+-------+
| user  |
+-------+
| !: %\ |
+-------+
1 row in set (0.00 sec)

Apparently that's valid.

We have lots and lots of old code using the existing DSN and we must not break it. To support that kind of username, we have to signal that a new kind of DSN is used.
I thought about a leading colon for base64 encoded username and password, but empty usernames are ok, too.

mysql> CREATE USER '';
Query OK, 0 rows affected (0.00 sec)

mysql> select user from mysql.user where user = '';
+------+
| user |
+------+
|      |
+------+
1 row in set (0.00 sec)

The options as I see them:

  1. signal that a new DSN is used by a leading ":"
    The new DSN might use base64 encoding or somesuch, that part is open.
    • ugly
    • breaks existing code for users with empty name but using a password
  2. add an alternative Open identifier ("mysql2") using another DSN
  3. wontfix

@julienschmidt @methane what do you think?

@julienschmidt
Copy link
Member

@csyangchen if we would start from scratch, we would certainly choose to use just a standard conforming URL as DSN.
However, we currently have about 4000 repository clones daily. I assume the outcry would be huge if we introduce a breaking change here. I assume that we would break thousands of programs. A breaking change is definitely not an option.

So @arnehormann, I would go with option number 2 or 3 (in now more than 5 years this driver exists, nobody ever complained about it until now). In case of number 2, I would however go with something semantically more meaningful than "mysql2" and then use the url package to parse the DSN string.

@arnehormann
Copy link
Member

let's go with wontfix, then. Adding a different Open identifier is its own can of worms...

@ans- ans- mentioned this issue May 30, 2017
5 tasks
@nemith
Copy link
Contributor

nemith commented Oct 20, 2017

So not a problem in 5 years and now 2 problems in less than 6 months. Any consideration to the other options? I hate to maintain a fork.

@julienschmidt
Copy link
Member

julienschmidt commented Oct 20, 2017

Starting with Go 1.10 there will be an alternative way to open connections using a Connector (we will use our Config struct to implement the Connector interface): https://tip.golang.org/pkg/database/sql/#OpenDB
This would avoid all of these encoding problems.

See also #671

@julienschmidt julienschmidt changed the title ParseDSN don't support special char ParseDSN doesn't support usernames containing colons Oct 20, 2017
@julienschmidt
Copy link
Member

We should at least document the current situation and propose workarounds.

@julienschmidt julienschmidt reopened this Oct 20, 2017
@nemith
Copy link
Contributor

nemith commented Oct 20, 2017

Waiting until Go 1.10 seems like a good compromise.

Documentation would be great. I spent a lot of time pulling out hair. I already implement my own driver that does some preliminary lookups against our service discovery system, so mysql DSNs are never interacted with directly.

@nemith
Copy link
Contributor

nemith commented Oct 20, 2017

In fact if Connector interface were to be implemented i could use it directly today even on older versions of go since I already create my own Driver and just wrap it?

I guess the concerns could be that the interface signature could change before the 1.10 freeze, but that is pretty low.

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

No branches or pull requests

5 participants