There are many occurances of 

if ($strPrefix === FALSE)
  do something;

In almost all cases, $strPrefix is NULL, and there is another variable named $strprefix in use. The most problematic of these cases in in _phplist_get_prefix, where the test for $strPrefix === FALSE never succeeds, so the prefix is never set and NULL is always returned.

In this patch I do away with all of this checking, because why assume there will always be a prefix? As far as I can tell (I'm new to phplist), there is a prefix by default, but no need to use one if you don't want to.

CommentFileSizeAuthor
#1 strprefixpatch.txt4.18 KBpaulbeaney
phplist_strprefix.diff3.7 KBDave Cohen

Comments

paulbeaney’s picture

paulbeaney commented
StatusFileSize
new strprefixpatch.txt4.18 KB

Hi,

Thanks for your input to this module.

First of all, I admit to being guilty on all counts of having introduced a mixed-case variable into the module. I forgot to run the module through a code checker before the last commit and hence I didn't spot it (along with some tabs that crept in too...). I will correct this the next update I do.

Secondly, to address your question about all the prefix checks - I found to my cost that they are VERY necessary. Why ? Quite simply, because when you install the module for the first time, there are no configuration settings, so unless you avoid running certain parts of the code (based on a simple is-the-phplist-password-set test), you will quickly find that a number of your pages give major Drupal database error messages. For the (very) slight extra overhead of these checks, I prefer to do them and keep the site as error-free as possible.

Technically, there is always a prefix to find, even if it happens to be an empty string. The _phplist_get_prefix() code was definitely flawed as the static variable has a value of NULL when it is first created, and as you correctly pointed out, I was testing for FALSE. Not a good plan...

I propose the attached patch as a resolution to this problem, keeping the extra $strprefix checks in place. If you could check it works ok for you I'd be grateful. I have tested is on phplist_ and "no prefix" databases.

Regards,

- Paul

paulbeaney’s picture

paulbeaney commented
Status: Needs review » Fixed

No feedback so adopting patch in post #1.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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