This hasn't been used since Drupal 6, but it's still declared as global in bootstrap.inc, we should just remove it from that line.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

karschsp’s picture

Assigned: Unassigned » karschsp
FileSize
788 bytes

Simple enough. Let's see if I remember how to roll a patch.

karschsp’s picture

Status: Active » Needs review

Darn. Setting to "needs review"

joris_lucius’s picture

Not much to test here.
Did a search for $db_url and did not find any others.

@karschsp not sure if you tried that, just making sure

longwave’s picture

The only other mention of db_url is in install.core.inc, and I think this should stay for security:

    // Do not install over a configured settings.php. Check the 'db_url'
    // variable in addition to 'databases', since previous versions of Drupal
    // used that (and we do not want to allow installations on an existing site
    // whose settings file has not yet been updated).
    if (!empty($GLOBALS['databases']) || !empty($GLOBALS['db_url'])) {
      throw new Exception(install_already_done_error());
    }

(see also #1816124: Fix installer PHP code execution issues from SA-CORE-2012-003 (and backport anything to 7.x-dev as necessary))

mitron’s picture

Longwave in •4 is correct. There are only two mentions of db_url. One is removed by the patch. The other should remain in the code. The patch appears to be correct and does not appear to create any other potential problems.

longwave’s picture

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

Status: Reviewed & tested by the community » Needs work

That code only works because db_url is previously exported as global in drupal_settings_initialize(), so it's not going to be correct if that's removed from bootstrap.inc

We don't support putting a Drupal 6 settings.php into a Drupal 8 site, not for upgrades or any other reason, so I think it's OK to remove both here.

mitron’s picture

OK, so the value for $db_url would come from including settings.php inside drupal_settings_initialize(). If it is not made global in the same function, it would never be in $GLOBALS as shown in #4, thus leaving it there is pointless. If it is OK to remove the first reference, it therefore must be OK to remove the second.

mitron’s picture

Status: Needs work » Needs review
FileSize
1.4 KB

Proposed patch.

longwave’s picture

Status: Needs review » Needs work

The comment in install.core.inc still mentions 'db_url'.

mitron’s picture

Status: Needs work » Needs review
FileSize
1.56 KB

With comment removed.

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, remove_db_url-1881992-11.patch, failed testing.

mitron’s picture

Status: Needs work » Needs review
Issue tags: +Novice

#11: remove_db_url-1881992-11.patch queued for re-testing.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me now.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

Automatically closed -- issue fixed for 2 weeks with no activity.