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

MySqlConnectionStringBuilder.ConnectionString depends on order in which properties are set #1217

Closed
bgrainger opened this issue Sep 14, 2022 · 5 comments · Fixed by #1219
Closed
Milestone

Comments

@bgrainger
Copy link
Member

var builder1 = new MySqlConnectionStringBuilder { Server = "localhost", UserID = "root" };
var builder2 = new MySqlConnectionStringBuilder { UserID = "root", Server = "localhost" };
Console.WriteLine(builder1.ConnectionString);
Console.WriteLine(builder2.ConnectionString);

produces:

Server=localhost;User ID=root
User ID=root;Server=localhost

It's unexpected that the serialized connection string would depend on the order in which C# properties are set.

This may be an artifact of some logic in the base DbConnectionStringBuilder class, possibly in an attempt to preserve connection strings set via the string constructor? (Although normalization of property names and punctuation does occur; see example below.) Or it could be an unintentional side-effect of using a Dictionary to store the options, which tends to return Keys in the order in which they were inserted, although this is certainly not guaranteed by the implementation.

var builder3 = new MySqlConnectionStringBuilder("Server=localhost;User ID=root");
var builder4 = new MySqlConnectionStringBuilder("User ID=root;Server=localhost");
var builder5 = new MySqlConnectionStringBuilder("uid = \"root\" ; Server = localhost");
Console.WriteLine(builder3.ConnectionString);
Console.WriteLine(builder4.ConnectionString);
Console.WriteLine(builder5.ConnectionString);

produces:

Server=localhost;User ID=root
User ID=root;Server=localhost
User ID=root;Server=localhost

Since option key names and punctuation are already normalized by the implementation, it makes the most sense to also output the options themselves in a consistent order; OrdinalIgnoreCase would be simplest.

@bgrainger
Copy link
Member Author

bgrainger commented Sep 14, 2022

Microsoft.Data.SqlClient.SqlConnectionStringBuilder and Microsoft.Data.Sqlite.SqliteConnectionStringBuilder normalize the order of options. Both of these types perform the ordering by using a hard-coded list of option names, not just by string comparison.

https://github.com/dotnet/SqlClient/blob/7d348eb0c985b34ce5b76a9330c57bc1015c93ff/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.cs#L1755

https://github.com/dotnet/efcore/blob/5c46563540143f232bdf4968073f35547dbb5dbd/src/Microsoft.Data.Sqlite.Core/SqliteConnectionStringBuilder.cs#L131-L132

NpgsqlConnectionStringBuilder has the same behavior as MySqlConnectionStringBuilder, as does MySQL Connector/NET.

@ejball
Copy link
Contributor

ejball commented Sep 14, 2022

Related: dotnet/runtime#834

@bgrainger
Copy link
Member Author

OrdinalIgnoreCase would be simplest.

But may not be what people expect. It's very common (for MySQL connection strings) to see Server as the first option, e.g., https://www.connectionstrings.com/mysql/ and https://mysqlconnector.net/connection-options/ (and https://mysqlconnector.net/overview/configuration/ although it says host). We could follow SqlClient's approach and hard code the list of keys in the "best" order.

@bgrainger
Copy link
Member Author

dotnet/runtime#834 (comment) raises a point that's relevant to MySqlConnector; because MySqlConnectionStringBuilder.ConnectionString is used as a key into a shared dictionary of connection pools, the following code creates four separate pools:

using (var connection = new MySqlConnection("server=localhost;userid=root;password=pass"))
	connection.Open();
using (var connection = new MySqlConnection("server=localhost;password=pass;userid=root"))
	connection.Open();
using (var connection = new MySqlConnection("userid=root;password=pass;server=localhost"))
	connection.Open();
using (var connection = new MySqlConnection("password=pass;userid=root;server=localhost"))
	connection.Open();

MySqlConnector should detect that these are all equivalent connection strings and map them to the same pool. While fixing the order of options in MySqlConnectionStringBuilder.ConnectionString is not the only way to fix that bug, it's a straightforward way to address the problem.

@bgrainger bgrainger added this to the 2.2 milestone Sep 15, 2022
bgrainger added a commit that referenced this issue Sep 15, 2022
Signed-off-by: Bradley Grainger <bgrainger@gmail.com>
bgrainger added a commit that referenced this issue Sep 15, 2022
Signed-off-by: Bradley Grainger <bgrainger@gmail.com>
bgrainger added a commit that referenced this issue Sep 15, 2022
Signed-off-by: Bradley Grainger <bgrainger@gmail.com>
@bgrainger
Copy link
Member Author

Fixed in 2.2.0.

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

Successfully merging a pull request may close this issue.

2 participants