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

Umbraco installer double-encodes connection string password #4772

Closed
glombek opened this issue Feb 27, 2019 · 4 comments · Fixed by #4820

Comments

@glombek
Copy link

commented Feb 27, 2019

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

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

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

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

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

@iofsauron

This comment has been minimized.

Copy link

commented Feb 28, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Mar 2, 2019

PR: #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: #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.