Umbraco-cms: Umbraco installer double-encodes connection string password

[glombek]

When entering database connection details in the Umbraco installer, the connection string that ends up in web.config is double-encoded and therefore doesn't work.

Reproduction

Found in 7.11.1

Steps to reproduce

Set up a Azure SQL database with a user with an ampersand (&) in the password.
On a fresh Umbraco install, run the /install installer
Enter details on the first screen, click Customise.
Enter the Azure SQL db details
Click Next
YSOD screen showing that the user could not log in.

If you check Web.config, the connection string will contain a double-encoded password.

[nul800sebastiaan]

That is unfortunate, in the past we actually fixed the installer to ENCODE ampersands, otherwise there were errors. I can't imagine there's an actual good solution to this, we could first decode the password and then encode it again, possibly, but that might lead to other problems along the way. What do you think? It might work..

[nul800sebastiaan]

I've marked this as "Up for grabs" so that you or someone else coming along could create a pull request for it.

[iofsauron]

Line 56 in DatabaseConfigureStep.cs appears to be the culprit. When we remove the encoding the database connection string is no longer doubled encoded however we have run out of time here at the Uduf hackathon to fix and test what that change breaks, then submit a PR

[EwneoN]

PR: https://github.com/umbraco/Umbraco-CMS/pull/4820

Ok so after continuing on from the work Bec and I did at Uduf, I can confirm that removing the manual encoding of ampersands and other values in the password does not affect the DB connection string creation for any of the offered database types. I have left the escaping of single quotes though as this is actually required.

I have struggled to reproduce why this was an issue in the first place as the details on the pull request(PR: https://github.com/umbraco/Umbraco-CMS/pull/1335) that brought the changes in do not really explain the reasoning as to why this XML encoding was originally added. The original commit seems to be about handling semi colons in passwords which was correct, but the adding of the XML encoding is not actually required as this is already handled by the XDocument code when saving the connection string.

https://docs.microsoft.com/en-us/dotnet/api/system.data.sqlclient.sqlconnection.connectionstring?redirectedfrom=MSDN&view=netframework-4.7.2#System_Data_SqlClient_SqlConnection_ConnectionString