Uninstall of protector does not remove the preload

[skenow]

The protector preload is still active after deactivating the module. Uninstalling the module does not remove the preload, either.

[skenow]

And, reinstalling the module does not add the preload back in if it has been manually removed.

[fiammybe]

To my knowledge, there is no real mechanism in the core at the moment to deactivate preloads, unless you want to physically remove them from the file system. The same for 'installing' preloads : a module can copy the files from one of his own folders to the preload folder, but in the best of cases that should be done by the core, and not by every module separately. Also, if the core offers this, preloads can be handled without being dependant on a module

[MekDrop]

I'm not sure if this is bug or feature but definitely not a regression.

[skenow]

@fiammybe - if a module adds a preload during its installation, it needs the mechanism to remove it on uninstall (and the ability to reinstall later)

@MekDrop - I added the regression label because the installation script in this branch has been modified to not install this module if PHP >= 7. The comments in the code suggest this module could be removed completely. In which case, this has to be dealt with.

[MekDrop]

Problem is that in previous versions same result could be achieved if you remove protector module from modules before installation. Sadly preloads right now doesn't work good enough in both versions.

I think for current case it would be easier for us just remove protector.

[fiammybe]

A module shouldn't have the responsibility of managing the preloads of the platform, that is a responsibility of the core in my opinion. The core doesn't have a good management of preloads, for site-wide preloads it looks if a file is there, and executes it, for module-specific preloads, you even have to add extra code to make it work. The protector module is not compatible with PHP7, both on the level of database interactions and code structure, and has been unmaintained for a few years now by the original author. If 1.4 is to be released fairly quickly, not installing protector in PHP7 looks like the only option.

1.4 is meant to be 1.3 for PHP7, so if functionality isn't in 1.3, it should not be added to 1.4 as a general rule.

[skenow]

We have been installing Protector with the core of ImpressCMS since 2009 (10 years ago - version 1.2). So, the majority of sites (make that every site) has this module installed. When they upgrade to 1.4 and then switch their host to PHP7 - the primary reason for this version - their sites will stop working.

We have 2 options -

1. Get Protector to work on PHP7 (not that difficult, really. I'm working on it)
2. Figure out how to properly uninstall a module that has preloads (it can be done - and should be done - by the module. After all, it added it) and execute the uninstall during the 'upgrade'. ("Oh - by the way, your site will run on the latest version of PHP, but it will be less safe")

[skenow]

In the uninstall function, there is a process to attempt removing the preload

trust_path/modules/protector/onuninstall.php -

if(defined(ICMS_PRELOAD_PATH) && file_exists(ICMS_PRELOAD_PATH.'/protector.php') && function_exists('icms_deleteFile')){ icms_deleteFile(ICMS_PRELOAD_PATH.'/protector.php'); } There is no error checking or logging at this point. The function has been deprecated so we can improve on a few things.

[fiammybe]

totally agree, error checking and logging should be added.

[skenow]

Argh - a simple error. The check should be define('ICMS_PRELOAD_PATH'), not define(ICMS_PRELOAD_PATH)

[skenow]

I believe the code merged in #492 deals with this and this can be closed. Best if someone can review and verify