[DX] Switch to a simplified array syntax for database connection information in settings.php

[olafgrabienski]

I just tried to install Backdrop 1.5 in a local AMPPS environment. During installation, after being on the language page, I got the message

Fatal error: Class 'DatabaseTasks_' not found in {...}/core/includes/install.inc on line 1362

and wasn't able to continue the installation.

Line 1362 concerns the database driver. Googling something like drupal error database driver led me to a comment on d.org regarding Backup and Migrate about a problem with special characters in the database password string. I had a look in my settings.php, noticed a forward slash in the password string, changed the database password in the relevant places and was able to continue the installation.

Are forward slashes (and possibly particular other special characters) in the database password string not supported by Backdrop for a certain technical reason? If so, we should document it somewhere. If not, can we fix that?

ADVOCATE: @yorkshire-pudding

[Dinornis]

I experienced a similar issue with a password that included special characters, the only difference was that I did not see an error message, just a blank page during install. Not sure what would be the best way to address this.

[olafgrabienski]

Today I got the same, or very similar, issue again. Installation of Backdrop failed, displaying the general message "The website encountered an unexpected error. Please try again later."

When I went back to the site URL without the installer related query strings, I got a more specific notice:

Notice: Undefined index: driver in {...}/core/includes/database/database.inc on line 1718

As in the first post above, it looks again like a database driver related issue. (Maybe sort of misleading, because the driver is the first element of $database in settings.php.) I was however able to install Backdrop with a new database using a new password, and I guess that worked because there were no 'problematic' characters in the string.

So I ask again: are particular special characters not supported in the database password string?

[LeeteqXV]

I just ran into this on a test installation of 1.6.3, where the password contained a semicolon (;) and a hash (#) character on two different/adjacent locations in the password. I replaced those two with underscores (so that at least it had 1 special character in it)(using the cPanel tool to update/change existing dbuser passwords), reloaded the error page, and the installation continued without any further problems.

[olafgrabienski]

Documenting for testing purposes: characters mentioned in the posts above that might be problematic:

• forward slash: /
• semicolon: ;
• hash sign: #

Feel free to add more.

[daggerhart]

This is because the bootstrap is using parse_url() to load this string into an array, so anything that is part of the url schema is problematic.

Edit: Found documentation related to using the array format in settings.php. Tested and it works. This should allow special characters without issue.

---

Edit 2: Deleting all my custom code non-sense. I think we should instruct people to use the array syntax in settings.php.

[olafgrabienski]

@daggerhart Do you suggest to change settings.php, so that the default would be to use the array format instead of the simple database configuration?

[laryn]

I just hit this using an autogenerated password. It was a big pain to sort out as things had already been partially installed and I had to:

• reset the password for the database
• edit settings.php to update the password there
• reload and still got errors so I decided to start fresh and replaced settings.php with the default version of that file
• began to reinstall then got a fatal error because config directories weren't empty
• emptied those, reinstalled and now I'm getting Call to undefined function field_attach_load() so I'm planning to just nuke the whole thing and start fresh

[daggerhart]

@olafgrabienski sorry, i missed your message to me.

Yes, if we change to using the array format during settings.php generation, we'll avoid users running into issues like @laryn just experienced.

I don't know of any great reason to using the single string/url format. As a user, I think it is much harder to understand because you have to be somewhat familiar with authentication based url syntax and avoid its special characters.

Whereas the array syntax is very clear about what each value is, and (as far as i know) doesn't have any limitations concerning special characters.

[olafgrabienski]

I don't know of any great reason to using the single string/url format [for database configuration in settings.php].

@quicksketch @jenlampton To fix errors during installation due to special characters in the database password: Do you think it's possible to change the database configuration in settings.php, so that it uses the array format instead of the single string format? Or is there a particular reason why Backdrop has to use the single string format?

[jenlampton]

Or is there a particular reason why Backdrop has to use the single string format?

@olafgrabienski backdrop does not need to use the single string format, the array format from Drupal 7 will work as well.

[olafgrabienski]

Thanks @jenlampton ! To change settings.php in such an essential part seems a big change to me. Does it make sense, and are there caveats? I'd appreciate some more feedback from experienced Backdrop developers before we make a PR!

[jenlampton]

There's no need for a PR, backdrop already works this way. On your site: copy the array syntax from a D7 settings.php file and see if that fixes the problem. If so we can close this issue.

[laryn]

@jenlampton If we want it to use the array syntax by default on new installs (to avoid the special characters in the MySQL password causing a problem) we'd at least need a PR to change the default settings.php file, wouldn't we? And maybe some fixes where Backdrop saves settings during installation?

[jenlampton]

We don't want to use the ugly array syntax by default. The simper syntax was chosen because all people will understand it instantly. It's a universal syntax, and will benefit 100% of backdrop users. For the minority of sites that still need to use the array syntax for any of the various edge-cases, that Is an option too.

…

On Jun 7, 2017 10:18 AM, "Laryn - CEDC.org" ***@***.***> wrote: @jenlampton <https://github.com/jenlampton> If we want it to use the array syntax by default on new installs (to avoid the special characters in the MySQL password causing a problem) we'd at least need a PR to change the default settings.php file, wouldn't we? And maybe some fixes where Backdrop saves settings during installation? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2231 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAYSRxL2J6SSgrEuKRqzx8MiETXrlS2Jks5sBttsgaJpZM4KFxLV> .

[laryn]

@jenlampton Can you think of another solution to the special character issue caused by the single line syntax? (ie. what this thread is about)

I don't think I'd call that an 'edge case' and expect new users who run into this to be able to figure out why they can't even install Backdrop in the first place. For example, using the cPanel "generate password" functionality on a new MySQL database user will almost always include a special character like this.

[jenlampton]

It's important that we avoid using a crazy backdrop-specific database connection syntax (by default) because that's the first thing people will see in settings.php. New users will see that first when setting up Backdrop, and that creates a poor first impression too.

Another thing we should do is make the installer UI be smarter about special characters in the database password. If a user enters one into the UI during installation, the installer should switch to using the crazy syntax when it writes to settings.php, or escape special characters when necessary. At the very least, it could throw an error or warning message letting people know what's up rather than just crashing the site. :(

Since what we're using now is a universal syntax, maybe we could look at what other all the other systems are doing about it, for reference?

[laryn]

I'm not sure what's so bad about the array format (I actually think it's clearer). How often will the 80% need to open settings.php anyway following a successful installation via the browser interface?

That said, I don't really have strong opinions as long as the issue gets solved.

[klonos]

Sorry @jenlampton, but I'm with the rest of the people on this one...

1. I find that the array syntax is more self-explanatory than the URL syntax. Mainly because it uses placeholders for the parts in a way that resembles field label/input pairs. IMO this would help beginner devs more.
2. The array syntax eliminates this problem here, which is a showstopper and causes a very bad UX for beginners trying to install Backdrop for the first time. Especially in very commonly-used environments like cPanel.
3. "Backdrop values site builders over coders". Very few will need to directly mess with the settings.php file (which is code) in the first place. Or at least that's how things should be. Things are meant to be handled by our installer UI.

Now, if we can find an easy way to make the password field smart enough to detect and escape problematic characters before writing to the settings.php file, then I'm equally OK with that.

[klonos]

...to sum things up:

It's important that we avoid using a crazy backdrop-specific database connection syntax (by default)...

I agree.

...because that's the first thing people will see in settings.php.

These people will most likely be developers. Site builders do not mess with the settings.php file. At least beginner site builders that expect things to work from the UI.

New users will see that first when setting up Backdrop, and that creates a poor first impression too.

Not poorer impression than a "broken" installer that "doesn't work" and that leads you to have to fiddle with code.

[docwilmot]

Agreed.
We need to either default the array syntax, or find/build an alternative to parse_url().

[jenlampton]

1. Top priority: If we're talking about the site-builder experience then the format we use in the settings.php file is irrelevant, since the site-builder won't be looking at it. We should fix the installer to either throw a validation error if a password is entered that won't work, or, ideally, deal properly with whatever password is provided. It's not a good idea to solve UX problems by creating DX problems when it's not necessary.

2. Our second priority is for the beginner coder (or beginner Backdrop user), and for them it's very important that we not use the array syntax from D7. I have a feeling all of you are too advanced to understand what it feels like for a non-developer to look at settings.php in Drupal 7 for the first time. I've trained hundreds of people on Drupal. The settings.php file shatters people's confidence - it deters them from thinking that they can do anything at all with Drupal's code.

I'm not sure what's so bad about the array format

• There are 187 lines of documentation needed to explain this syntax. Starting right off with 187 lines of "help" on how to hook up the database says "This software is so complicated that you can't even understand the first step without reading this epic". That is not the first impression we want to give people.
• Nested arrays are horribly confusing to people who don't know what arrays are (or understand the array syntax in PHP).
• Why is the first key "default" when you only have one database? Why is the second key also "default" when you still only have one database? These are unnecessary abstractions for the majority of users who only have a single database. In order to use the existing array syntax we require that people understand concepts that are irrelevant to the their own use case. That's not in line with backdrop's philosophy.

I think we may all be missing the point here. Changing the syntax back to the mess from D7 is not the only answer.

If we want to fix the UX of the installer, good, let's do that! :)

If we want to improve the DX of the settings.php file, fine, let's do that too! But let's not move backwards. Let's find a new solution that doesn't have all the same problems that we've already solved.

Not poorer impression than a "broken" installer that "doesn't work" and that leads you to have to fiddle with code.

These two things are unrelated. We don't need to break the DX to fix the UX :) Let's fix all the things!

[docwilmot]

@jenlampton I think the 187 lines of documentation still exist; we just moved them to our docs site. 😄
Here's the current settings for dbases:

/**
 * Database configuration:
 *
 * Most sites can configure their database by entering the connection string
 * below. If using master/slave databases or multiple connections, see the
 * advanced database documentation at
 * https://api.backdropcms.org/database-configuration
 */
$database = 'mysql://user:pass@localhost/database_name';
$database_prefix = '';

Here's what it would look like:

/**
 * Database configuration:
 *
 * Most sites can configure their database by entering the connection details
 * below. If using master/slave databases or multiple connections, see the
 * advanced database documentation at
 * https://api.backdropcms.org/database-configuration
 */
  $database = array(
    'driver' => 'mysql',
    'database' => 'databasename',
    'username' => 'username',
    'password' => 'password',
    'host' => 'localhost',
    'prefix' => '',
  );

I dearsay I think the latter is certainly less scary.

In either case if we continue to use the string format, we must concede that users will have issues with password generators OR they will get nasty exceptions. Or we come up with an alternative to parse_url(). Frankly I'm not seeing many other options.

[jenlampton]

I think the 187 lines of documentation still exist; we just moved them to our docs site.

yes, because they aren't necessary anymore for people reading the settings.php file, thanks to our new cleaner syntax ;)

if we continue to use the string format, we must concede that users will have issues with password generators OR they will get nasty exceptions.

I gave two examples above of how this can be avoided...

Frankly I'm not seeing many other options.

Here's a third option, what WP does: https://codex.wordpress.org/Editing_wp-config.php

[docwilmot]

I gave two examples above of how this can be avoided

Sorry missing those, can you explain? Solutions are always a good thing.

[jenlampton]

Here's what it would look like:

/**
 * Database configuration:
 *
 * Most sites can configure their database by entering the connection details
 * below. If using master/slave databases or multiple connections, see the
 * advanced database documentation at
 * https://api.backdropcms.org/database-configuration
 */
  $database = array(
    'driver' => 'mysql',
    'database' => 'databasename',
    'username' => 'username',
    'password' => 'password',
    'host' => 'localhost',
    'prefix' => '',
  );

This example looks pretty clean and tidy... Does this work for backdrop already? Can we test it real quick?

[jenlampton]

Sorry missing those, can you explain?

If a user enters a password with special characters into the UI during installation, the installer should 1) switch to using the array syntax when it writes that info into settings.php, or 2) properly escape special characters (like slashes, using URI hex encodings) when necessary.

[yorkshire-pudding]

I noticed that this wasn't working for CLI install. I have added these in as #hidden fields on the form. If there is a suitable source for these to be dropdown and we can also consider validation (i.e. ensure charset and collation are compatible) then could make configurable in the UI (and CLI) (@izmeez - I'm trying to work it towards the configurable element of your request at #6711)

To make configurable in the CLI, we would need to either move core/scripts/install.sh to using individual parameters for DB config (preferred) or stick with db url and add in additional options

Bee can (and will) be adapted as required.

EDIT: And now cSpell passes ????

EDIT: if you want to test on something like Lando or another system that provides the db settings at server level, you may need to add this line to your settings.php:

if (isset($_SERVER["BACKDROP_SETTINGS"])) unset($_SERVER["BACKDROP_SETTINGS"]);

[izmeez]

I'm having some difficulty understanding the instructions on testing this. I guess I may just have to experiment with it in conjunction with the PR.

EDIT: if you want to test on something like Lando or another system that provides the db settings at server level, you may need to add this line to your settings.php:

if (isset($_SERVER["BACKDROP_SETTINGS"])) unset($_SERVER["BACKDROP_SETTINGS"]);

[yorkshire-pudding]

I'm having some difficulty understanding the instructions on testing this. I guess I may just have to experiment with it in conjunction with the PR.

EDIT: if you want to test on something like Lando or another system that provides the db settings at server level, you may need to add this line to your settings.php:

if (isset($_SERVER["BACKDROP_SETTINGS"])) unset($_SERVER["BACKDROP_SETTINGS"]);

@izmeez - in Lando (and possibly in other docker-based systems) the database settings are never written to settings.php and the installer skips the database step unless that line is added to settings.php. You may not need this in your local environment.

[quicksketch]

I posted a review to the PR: backdrop/backdrop#4567 (review)

Overall, looking great!

[yorkshire-pudding]

Thanks @quicksketch - I learned a new bit of syntax and have implemented those changes. The change from implode to serialize enabled a small simplification which I've also added.

I have tested this through the UI and through bee (which uses the install.sh script) with a complex db password ('P@s$/W0rd#;') and both installed without a problem.

[quicksketch]

As @yorkshire-pudding is already advocating for this issue and this is close to completion, this should be in the 1.30.0 milestone (as I just moved it).

[quicksketch]

@yorkshire-pudding I tried out this PR and ran into some issues with some edge-case handling.

We have a special check in DatabaseTasks::connect() (in .install.inc) that detects if localhost or 127.0.0.1 is used, and if the connection fails, it tries swapping one for the other. This helped with a common database misconfiguration. This was added in #496.

But that check conflicted with this change because it tried to automatically rewrite settings.php when it does the check. And then the installer form tried to rewrite it a second time, and it would result in invalid PHP because the $database variable changed its number of lines from the first rewrite.

So to avoid going down the rabbit hole of needing to expand backdrop_rewrite_settings(), I pushed a commit to the PR that remove the automatic rewriting. Instead, a message is shown during the installation:

The entire localhost/127.0.0.1 swapping seems to be less of an issue than it was previously, due to the increase in tools like DDEV and Lando. So removing the automatic attempt fix the issue and request the user set a different value seems okay to me.

@yorkshire-pudding Please take a look at my changes (made directly on your PR) and see if you'd suggest anything different.

[yorkshire-pudding]

@quicksketch - your changes make sense. I have no way to test this 'host' scenario.
@olafgrabienski - thanks for your suggestion, I have clarified where they need to update.

[quicksketch]

I merged backdrop/backdrop#4567 for 1.30.0 that should fully fix the original problem by avoiding needing to URL encode values entirely. This original issue was mostly solved back in 2019 by @jlfranklin (#2231 (comment)). A big thanks to @yorkshire-pudding for finishing the job!

backdrop/backdrop@c53e13d by @yorkshire-pudding, @klonos, @izmeez, @quicksketch, @olafgrabienski, and @bugfolder.

This issue is code complete but we need to both update the documentation page at https://docs.backdropcms.org/documentation/database-configuration and write a change record.

[yorkshire-pudding]

I'll start the Change record first.

[yorkshire-pudding]

I've added a change record at https://docs.backdropcms.org/change-records/database-configuration-can-now-be-stored-in-a-simple-array

I've spotted that @quicksketch has already published a change record for 1.30.0 so I have published this. Edit if required.