Use HTMLPurifier from composer instead of packages

[colemanw]

Overview

Switches to using composer for this library
Depends on civicrm/civicrm-packages#331

Technical Details

Last time we tried this it didn't go so well. But @seamuslee001 thinks this time it might be different.
See https://lab.civicrm.org/dev/core/issues/976
and #14277

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

I'm wondering why we do this at all. Why not just rely on the composer autoloader?

[colemanw]

Ditto, couldn't we just leave this blank and let the autoloader do its thing?

[seamuslee001]

@colemanw no I don't believe so because the IDS doesn't really understand things in composer land

Also this needs to use the ['civicrm.vendor'] path and also that will just print literal ['civicrm.root'] not the calculated path I believe.

[colemanw]

OK cool. The ['civicrm.vendor'] must be new since the last time this was attempted. I was just copying the code from there.

I don't think it will print the literal string because of this:

https://github.com/civicrm/civicrm-packages/blob/7db3890327e396abc04138fdd54d5c05a827ec70/IDS/Monitor.php#L207-L213

[colemanw]

@seamuslee001 I've updated this to use [civicrm.vendor] and also updated the PR in /packages

[demeritcowboy]

I'm trying to test this out, and while I'm able to get IDS itself to trigger on bot-like urls, I can't get htmlpurifier to do anything. In particular it seems to only be used if you have html set in the config, and it isn't set by default:

https://github.com/civicrm/civicrm-packages/pull/331/files#diff-d6538ccd6b3071398f365f16ffbffc063e993e4091de8cd0204f5b0b34b7daabR27

and then core civi seems to compile a config but seems to be looking for ids_arguments in the menu.xml files and there aren't any that I see, and when I debug the $init config it generates it doesn't contain "html":

civicrm-core/CRM/Core/IDS.php

Line 66 in 4def3d9

$ids = new \IDS_Monitor($_REQUEST, $init);

So then this never happens:
https://github.com/civicrm/civicrm-packages/blob/9840ce786a6cb3a68cd4e50ecc4f8ba1e730cea9/IDS/Monitor.php#L340

I haven't checked for conflicts yet with the mentioned drupal module, but I'm wondering if I'm even testing this correctly. If I am doing it correctly, then if there were to be conflicts with the module, it seems like we could just remove any mention of HTMLPurifier if it's not being used anyway.

[demeritcowboy]

Interesting: https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_xmlMenu/#xml-ids

[colemanw]

Interesting indeed.

Tangentially, last time @seamuslee001 and I spoke about it we wanted to try getting rid of the HTML_Purifier_Path entirely and relying on the autoloader (which would in theory be preempted by the libraries_get_path() + require_once() hack we do for Drupal 7, so in theory that would make all the systems happy.

[demeritcowboy]

Did some more testing with Drupal 7, leaving IDS aside for the moment:

• Upgrade from 5.41 - I was worried maybe civi would crash as soon as you replaced the files and tried to visit the upgrade page.
• Upgrade from 5.41 with the drupal htmlpurifier module installed.
• It seems there's two methods of using the drupal htmlpurifier module - one that uses libraries api (libraries_get_path()) and one that doesn't have that installed. Tried both with the PR. I can see htmlpurifier does something on the drupal side, and visiting a page on the civi side with |purify in it also works.
  • I only did the upgrades without the libraries version.

Would like to see how this works in drupal 8 world. Will try to check that tomorrow.

[demeritcowboy]

It's a little difficult to test civi composer.json changes with drupal 8/9 because composer doesn't notice local changes in subpackages, so I had to kind of fake it and require it manually at the top level, so an upgrade experience might not be exactly the same but I can't think how it would be too different. On the plus side there's only one way to install the drupal htmlpurifier module with drupal 8. It all seems ok both with and without the module on both the drupal and civi sides, and doing upgrades.

The IDS stuff is a mystery to me how it ever worked triggered a call to htmlpurifier. I notice universe only has ids_arguments for the civicase extensions but only for json so again it wouldn't trigger htmlpurifier there.

Putting merge-ready if @seamuslee001 has any comments.