Differences in PDO and legacy query methods - failing because of HTML Purifier comments

I've been working on updating a few other modules for the move to 2.0 and PHP7/8 and ran into something in my testing - a database query was failing while using PDO and successful when using just mysql. Digging a little more, it is because of the call to the Protector MySQL Database method of checkSQL() from the icms PDO queryF method, which doesn't appear to be called by the legacy db class.

The reason the checkSQL fails is because of the HTML Purifier comments we add to the data before it is saved.

1. Why isn't this being triggered when using mysql?

2. How can we improve the logic without a lot of extra overhead?

There may be some things still in the module that is having this problem (Formulize) that will explain why the HTML Purifier comments are triggering this and not in other modules. But, that there is no problem when using mysql and only when using PDO brings up the question on why the data isn't being handled the same way.

I'm encountering in 1.4.x. This allows me to use PHP5.6 and switch between mysql and PDO. The modules aren't ready for PHP7, let alone dealing with the deprecated functions they still use.

My process has been to test with PDO and make sure there aren't any fatal PHP errors. That will tell me I'm close to being ready for PHP7. Then, I work on the deprecated functions and methods to make sure I'm ready for the later 1.4.x versions that remove many of those (and requires PHP7)

I did find if I comment out the line that inserts the 'input filtered' comment into the text, the error goes away.

Here's another puzzle related to this - 2 sites, same version of ImpressCMS, same version of PHP, both running on PDO, same modified version of the module, HTMLPurifier is enabled on both: 1 shows the error, the other doesn't. Fresh install of ImpressCMS (locally), fresh install of the module - both have errors.

I stand corrected - I started fresh (again) and installed my updated version of the module, and there have been no errors where I have seen them before. I have some work to do on my sites to resolve the real issue.

Continuing the journey to determine what the problem is, I thought I would see what happened when I just pointed 1 instance at the database of the other. Guess what? There's something different in the database that is affecting this.

I had overwritten the entire file/folder structure of the copy of the production site with the files from a fresh install and it still had the same problem. That's when I thought to change the db. I know it's not the tables for the module, because I've uninstalled and reinstalled and get the same errors.

Suspecting the HTML Purifier settings. I'm looking more closely at them. At an initial glance, they 'look; identical.

I was also thinking about preloads - there are differences between the sites. But, switching the db and using the production site's file base would have produced the errors if it was them.

So much for winter solstice - my day wasn't any shorter.

It's not the database or data.

Is this the right logic?

In icms_db_legacy_PdoDatabase

    public function queryF($sql, $limit = 0, $start = 0) {
        $result = FALSE;
        /* Use Protector's db layer protection against possible SQLi
         * This needs to be done for legacy queries, since PDO only offers
         * SQLi protection when you use bindParam and bindValue, and then
         * use prepare() and execute() on the statement
         */
        if (FALSE === icms_db_legacy_mysql_Utility::checkSQL($sql)) {
            return $result;
        }

The way I read it is if checkSQL is false, return false, which means the query failed (was blocked). The question then becomes what does checkSQL return if the SQL is clean?  Looking at it, it has more logic to call Protector's db protection method and there is no return value set - it only dies if there is a suspected injection.

I have tried a variety of things -

• Removing the 'input filtered' and 'filtered by HTMLPurifier' comments from the text at the beginning of the checkSQL method. It still fails
• If I don't add the comments in the filtering methods, it succeeds
• If I remove the '--' from the doubtful_needles array, it also succeeds.

I keep getting distracted by what I am seeing - looking at other modules to see how things might be handled differently, or we'd be seeing a lot of issues with this.