postgres db connection ... eliminate warnings [#39382]

It is Not a serious bug but certainly need to be paid attention to for robustness reasons.

In the db_connect function in includes/database.pgsql.inc file, I prefer to replace

  $conn_string = ' user='. $url['user'] .' dbname='. substr($url['path'], 1) .' password='. $url['pass'] . ' host=' . $url['host'];

with

  $conn_string = "";
  $conn_string .= isset($url['user']) ? ' user='. $url['user'] : '';
  $conn_string .= isset($url['path']) ? ' dbname='. substr($url['path'], 1) : '';
  $conn_string .= isset($url['pass']) ? ' password='. $url['pass'] : '';
  $conn_string .= isset($url['host']) ? ' host=' . $url['host'] : '';

Comment	File	Size	Author
#6	database.pgsql.inc_0.patch	1.46 KB	Steve Simms
#6
#5	database.pgsql.inc.patch-3	1.55 KB	Steve Simms
#4	database.pgsql.inc.patch-2	1.04 KB	Steve Simms
#3	database.pgsql.inc.patch-1	1.55 KB	Steve Simms

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Cvbge CreditAttribution: Cvbge commented 2 December 2005 at 22:53

Could you explain reasons for this change?

Comment #2

thinker CreditAttribution: thinker commented 5 December 2005 at 01:21

1) it will be always better to check variables before using it.

2) In case there is no password given in the settings.php, you will not receive any php notice in the log file.

3) Well structured.

Comment #3

Steve Simms CreditAttribution: Steve Simms commented 4 February 2006 at 21:15

Title:	postgres db connection	» postgres db connection ... eliminate warnings
Version:	4.6.3	» x.y.z
Assigned:	thinker	» Steve Simms
Priority:	Normal	» Minor
Status:	Active	» Needs review

File	Size
database.pgsql.inc.patch-1	1.55 KB

It would be good to eliminate the password notice on systems that don't require one. In PostgreSQL, all of those fields are optional, so it makes sense to only include them if they're set.

The change thinker suggested needs a bit of tweaking, though. The various components of $url are being filtered with urldecode(), which sets all of them.

I've come up with two possible solutions with patches, depending on how the committer wants to solve it. The first one's attached, and I'll add another follow-up with the second one.

I've confirmed that this works properly even when settings.php has $db_url = 'pgsql://';.

File	Size
database.pgsql.inc.patch-2	1.04 KB

Comment #5

Steve Simms CreditAttribution: Steve Simms commented 4 February 2006 at 22:27

File	Size
database.pgsql.inc.patch-3	1.55 KB

Here's a new patch, combining elements from the previous two, based on suggestions from cvbge.

Note that using pgsql:// generates a warning from parse_url(), but it does work. Any URL with a host will now be processed without any warnings, regardless of whether or not user, password, or database are present.

Comment #6

Steve Simms CreditAttribution: Steve Simms commented 4 February 2006 at 23:17

File	Size
database.pgsql.inc_0.patch	1.46 KB

One more patch. This cleans up the code by combining the two sections in the previous patch.

Again, based on feedback from cvbge on IRC.

Comment #7

Steve Simms CreditAttribution: Steve Simms commented 4 February 2006 at 23:20

Also, I added urldecode to the port string, since that was the only one without it, for completeness sake.

Comment #8

sammys CreditAttribution: sammys commented 17 August 2006 at 03:04

Version:

x.y.z

» 4.7.3

This patch is now 4.7 only. Can we get an up-to-date patch. Mail me once it's ready and i'll review it. We can integrate the changes with 4.8+ as well once the 4.7 version is committed.

--
Sammy Spets
Synerger
http://www.synerger.com