View Issue Details

IDProjectCategoryView StatusLast Update
0017172phplist applicationAll Otherpublic16-07-14 09:33
Reporterduncanc 
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Version3.0.6 
Target Version3.0.7Fixed in Version3.0.7 
Summary0017172: update mysql.inc to mysqli.inc
DescriptionIn the file admin/mysql.inc, the first few lines of function Sql_Connect look suspicious because of the indentation and lack of a '{' on the if statement.
The indentation suggests that the three statements should be within the same block.


function Sql_Connect($host,$user,$password,$database) {
  if ($host && $user)
    $compress = (empty($GLOBALS['database_connection_compression'])) ? 0 : MYSQL_CLIENT_COMPRESS;
    $secure = (empty($GLOBALS['database_connection_ssl'])) ? 0 : MYSQL_CLIENT_SSL;

    $db = mysql_connect($host, $user, $password, false, $compress | $secure);
  $errno = mysql_errno();
  if (!$errno) {
    $res = mysql_select_db($database,$db);
    $errno = mysql_errno();
  }
TagsNo tags attached.

Relationships

related to 0017187 resolvedmichiel SQL is not displayed when an error occurs 

Activities

michiel

01-05-14 01:46

manager   ~0053645

I'm not sure why you think it's suspicious. I think it was a git-contribution.

but also, mysql.inc is not used by default any longer, but mysqli.inc instead because mysql functions are deprecated.

we should also look more into mariadb really.

duncanc

01-05-14 07:01

developer   ~0053646

I think that you are mistaken about the default. The released config_extended.php still has this line:

$database_module = "mysql.inc";

and init.php still has

if (empty($GLOBALS['database_module'])) {
  $GLOBALS['database_module'] = 'mysql.inc';
}

But I am more puzzled by you saying that you don't see what might be wrong with the code, and also concerned that you seem to have accepted and applied a GitHub contribution without testing it.

michiel

01-05-14 16:36

manager   ~0053647

ah, that needs looking at. I think I'm using mysqli.inc in hosted. Mysql.inc should be removed.

I accepted the code, because it was well argumented and it does what it says, set the flags for secure and compressed on the mysql connection. They are just two "config" aliases.

duncanc

01-05-14 17:25

developer   ~0053648

I think that you are missing my point.

Only the first statement after the "if" is within the scope of the if. The second and third statements will always be executed. Because of the indentation that does not look to be the intent.

And it is a change from the previous code which had only a single statement, the mysql_connect statement, in the "if". Now the mysql_connect is always executed, even when host and user are empty. Presumably it is a valid use-case to not need host and user so this doesn't look to be a deliberate change.

michiel

03-05-14 02:46

manager   ~0053650

yes, looking at it closely, the compress and secure lines should have been before the condition not after.

for reference https://github.com/michield/phplist/commits?author=KyraD

duncanc

09-05-14 11:34

developer   ~0053675

Reviewing the mysqli.inc file, it doesn't have the test for host and user that is in mysql.inc

  if ($host && $user)

So maybe you thought that was not needed now.
But the change for compression and security needs to be applied to mysqli.inc.

duncanc

28-06-14 13:00

developer   ~0054065

The GitHub repository doesn't have the mysqli.inc file

https://github.com/michield/phplist/tree/master/public_html/lists/admin

Shouldn't the repository be release 3.0.6 plus whatever changes have been applied since then?