In MySQL database connections default to localhost, so it is normally ok to run MySQL commands without specifying that option and have it just connect to the localhost. In settings.php if you don't include the 'host' array value for a given database definition the following error will be given:
Notice: Undefined index: host in DatabaseConnection_mysql->__construct() (line 37 of includes/database/mysql/database.inc).
The site will still work, but the above error still displays. Adding the 'host' value makes the error go away.

My reason for opening this issue is that the error shouldn't display, it should default to transparently connecting to localhost as normal if the 'host' value is undefined.

Comments

damienmckenna’s picture

This is the line in question:

      $dsn = 'mysql:host=' . $connection_options['host'] . ';port=' . (empty($connection_options['port']) ? 3306 : $connection_options['port']);

If the 'host' value doesn't exist it'll end up with a string like the following:
mysql:host=;port=3306
With PHP 5.2.4 this works and will connect to the localhost database.

Given the PDO documentation does not define what is supposed to happen when this is blank or not present, the current result is that it behaves as if the value '127.0.0.1' was provided (note, I have not verified to see if it tries connecting via a socket or TCP/IP).

The questions are:

  • Should the value be optional for MySQL?
  • If yes, should anything be passed as a default?
  • If yes, should it pass "localhost" or "127.0.0.1" or an empty string (which works, see above)?
  • If not, should a more specific error message be displayed than just the PHP notice?
  • Either way, should the documentation be updated to cover this nuance?
AndrewBoag’s picture

I think that the host value should be required field. It's a core part of the database connection details and would not normally be left out inside an application db connection details.

It would be simple enough to add a default value which would stop this error.

c31ck’s picture

Version: 7.10 » 8.x-dev
Status: Active » Needs work

In any case the code you mention should check if $connection_options['host'] is set. If it's not set, it should either provide a sensible default (localhost) or it should show a clear error message. Furthermore, the documentation in default.settings.php isn't entirely clear on which parameters are required and which ones are optional.

AndrewBoag’s picture

Assigned: Unassigned » AndrewBoag
Status: Needs work » Needs review
StatusFileSize
new805 bytes

I have attached a patch that throws a warning message in the instance that the host is not defined.

swentel’s picture

Somehow, I feel tempted to mark this won't fix. The 'host' input field on the install screen is set to required, so there's no room for mistakes there. I'm not sure about drush, but that seems for more advanced users anyway. The patch feels like babysitting to me, I'd rather document this instead of adding an isset.

AndrewBoag’s picture

No disagreement from me. When I was writing the patch, it did seem like overkill.

c31ck’s picture

StatusFileSize
new827 bytes

How about defaulting to localhost if no value is set? The port value is handled the same way.

damienmckenna’s picture

@c31ck: +1 for that idea.

ramlev’s picture

Assigned: AndrewBoag » Unassigned
StatusFileSize
new877 bytes

Since the structure of files in drupal 8 has changed since the last patch was created, i have fixed it in this one.

ramlev’s picture

Status: Needs review » Reviewed & tested by the community
swentel’s picture

Status: Reviewed & tested by the community » Needs work

If we want to path this, we should check the postgres and sqlite driver as well. Also, other people should set issues to RTBC, not the patch author itself.

superspring’s picture

Status: Needs work » Needs review
StatusFileSize
new1.77 KB

This patch applies the 'default host' parameter to both Postgres and MySQL. Sqlite does not require a host parameter.

AndrewBoag’s picture

Issue summary: View changes
Status: Needs review » Closed (won't fix)

After a year or so away from this issue I see that things have sort of stalled.

The reality is that this is not really a critical bug in that if you have set up your Drupal DB settings properly, you won't be affected here.

If DB "host" ain't defined when it should be ... that is just sort of wrong and broken.

I'm closing this but please some-one reopen if they feel passionately about it.