[WIP] fix: Bye bye safehtml, bienvenue HtmlPurifier #4781

Closed
cerdic wants to merge 5 commits from refactor_texte_safety into master
cerdic commented 2 months ago
Owner

Refs: spip/spip#3926

PR en complément de spip/spip#5271

J'ai repris peu ou prou le code de la fonction safehtml() du plugin https://git.spip.net/spip-contrib-extensions/htmlpurifier/src/branch/master/inc/safehtml.php#L17

mais pas le reste du plugin.

La logique reste identique à ce qu'on faisait jusqu'ici. La fonction safehtml est utilisée avec parcimonie par le core et/ou textwheel pour cibler au mieux la sécurisation des contenus sans pénaliser la consommation des ressources.

Il pourra être utile de proposer un plugin "SafeHtmlPartout" qui en gros se plug derrière propre et typo et passe safehtml sur tous les contenus quand on est en mode parano. Ce n'est pas l'objet du core de faire ça

Refs: https://git.spip.net/spip/spip/issues/3926 PR en complément de https://git.spip.net/spip/spip/pulls/5271 J'ai repris peu ou prou le code de la fonction `safehtml()` du plugin https://git.spip.net/spip-contrib-extensions/htmlpurifier/src/branch/master/inc/safehtml.php#L17 mais pas le reste du plugin. La logique reste identique à ce qu'on faisait jusqu'ici. La fonction safehtml est utilisée avec parcimonie par le core et/ou textwheel pour cibler au mieux la sécurisation des contenus sans pénaliser la consommation des ressources. Il pourra être utile de proposer un plugin "SafeHtmlPartout" qui en gros se plug derrière propre et typo et passe safehtml sur tous les contenus quand on est en mode parano. Ce n'est pas l'objet du core de faire ça
cerdic added 1 commit 2 months ago
cerdic changed title from fix: Bye bye safehtml, bienvenue HtmlPurifier to [WIP] fix: Bye bye safehtml, bienvenue HtmlPurifier 2 months ago
Owner

En testant sous PHP 8.1 et 8.0 je tombe sur l'erreur suivante Fatal error: Array and string offset access syntax with curly braces is no longer supported in plugins-dist/safehtml/lib/html5/HTMLPurifier.standalone.php on line 3938 :\

Cela semble avoir été fixé dans le master depuis https://github.com/ezyang/htmlpurifier/pull/224

PS : la dernière version est la 4.14.0 cf https://github.com/ezyang/htmlpurifier/releases/tag/v4.14.0 alors que la version 4.10.0 embarquée dans la PR date de 2018.

En testant sous PHP 8.1 et 8.0 je tombe sur l'erreur suivante `Fatal error: Array and string offset access syntax with curly braces is no longer supported in plugins-dist/safehtml/lib/html5/HTMLPurifier.standalone.php on line 3938` :\ Cela semble avoir été fixé dans le master depuis https://github.com/ezyang/htmlpurifier/pull/224 PS : la dernière version est la 4.14.0 cf https://github.com/ezyang/htmlpurifier/releases/tag/v4.14.0 alors que la version 4.10.0 embarquée dans la PR date de 2018.
cerdic force-pushed refactor_texte_safety from 5a527f3983 to 2d479f291c 2 months ago
Poster
Owner

Je retravaillé la PR

  • lib en version 4.14.0 dans lib/htmlpurifier
  • j'ai séparé le commit de remplacement de lib de celui sur le code SPIP pour plus de lisibilité
  • j'ai modifié un peu la config tirée du plugin htmlpurifier de @g0uz qui n'autorisait que les iframes pointant vers le même domaine que le site
  • j'ai ajouté la possibilité de fournir un fichier safehtml/htmlpurifier.ini pour se faire une config de htmlpurifier aux petits oignons pour les utilisateurs avancés de la feature (qui doivent se compter au nombre de 1 :p)
Je retravaillé la PR - lib en version 4.14.0 dans `lib/htmlpurifier` - j'ai séparé le commit de remplacement de lib de celui sur le code SPIP pour plus de lisibilité - j'ai modifié un peu la config tirée du plugin htmlpurifier de @g0uz qui n'autorisait que les iframes pointant vers le même domaine que le site - j'ai ajouté la possibilité de fournir un fichier `safehtml/htmlpurifier.ini` pour se faire une config de htmlpurifier aux petits oignons pour les utilisateurs avancés de la feature (qui doivent se compter au nombre de 1 :p)
Owner

Top, plus d'erreur avec la mise à jour de la lib, juste quelques deprecated qu'on pourra leur remonter au fil de l'eau :)

Deprecated: Return type of HTMLPurifier_PropertyListIterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8340

Deprecated: Return type of HTMLPurifier_StringHash::offsetGet($index) should either be compatible with ArrayObject::offsetGet(mixed $key): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8458
Top, plus d'erreur avec la mise à jour de la lib, juste quelques deprecated qu'on pourra leur remonter au fil de l'eau :) ``` Deprecated: Return type of HTMLPurifier_PropertyListIterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8340 Deprecated: Return type of HTMLPurifier_StringHash::offsetGet($index) should either be compatible with ArrayObject::offsetGet(mixed $key): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8458 ```
Owner

Ok, j’ai les mêmes deprecated que @b_b

Ok, j’ai les mêmes deprecated que @b_b
cerdic added 3 commits 2 months ago
Poster
Owner

Alors quand même le lapin est sorti du chapeau : la version de la lib utilisée par le plugin HtmlPurifier est un fork pour le support de html5 et on avait donc perdu cela en intégrant la version 4.14.0 dist.

J'ai réintégré aussi bien que possible le patch via un diff/merge depuis la version du plugin, vu que la source https://gitlab.com/9hozt/html5purfier mentionnée par defcefbdce n'est visiblement pas/plus accessible.
(mais vas-y paye ton diff sur un fichier de 24000 lignes...)

A voir si ça nous plait plus d'intégrer https://github.com/xemlock/htmlpurifier-html5 qui est une surcouche du purifier, sans fork donc.

J'ai aussi pris le temps de regarder les tests unitaires en défaut suite au changement de lib, puisque le html généré n'est pas le même. Rien de grave, j'ai donc complété les tests unitaires pour accépter aussi la sortie proposée par HtmlPurifier quand elle divergeait de celle de safehtml.

Alors quand même le lapin est sorti du chapeau : la version de la lib utilisée par le plugin HtmlPurifier est un fork pour le support de html5 et on avait donc perdu cela en intégrant la version 4.14.0 dist. J'ai réintégré aussi bien que possible le patch via un diff/merge depuis la version du plugin, vu que la source https://gitlab.com/9hozt/html5purfier mentionnée par https://git.spip.net/spip-contrib-extensions/htmlpurifier/commit/defcefbdcedbf9f5ee533bd400523d31bfd04cb5 n'est visiblement pas/plus accessible. (mais vas-y paye ton diff sur un fichier de 24000 lignes...) A voir si ça nous plait plus d'intégrer https://github.com/xemlock/htmlpurifier-html5 qui est une surcouche du purifier, sans fork donc. J'ai aussi pris le temps de regarder les tests unitaires en défaut suite au changement de lib, puisque le html généré n'est pas le même. Rien de grave, j'ai donc complété les tests unitaires pour accépter aussi la sortie proposée par HtmlPurifier quand elle divergeait de celle de safehtml.
Owner

J'ai réintégré aussi bien que possible le patch via un diff/merge depuis la version du plugin, vu que la source https://gitlab.com/9hozt/html5purfier mentionnée par defcefbdce n'est visiblement pas/plus accessible.
(mais vas-y paye ton diff sur un fichier de 24000 lignes...)

huhu, la misère.

A voir si ça nous plait plus d'intégrer https://github.com/xemlock/htmlpurifier-html5 qui est une surcouche du purifier, sans fork donc.

ça me semble plus pérenne de prendre cette option :)

J'ai aussi pris le temps de regarder les tests unitaires en défaut suite au changement de lib, puisque le html généré n'est pas le même. Rien de grave, j'ai donc complété les tests unitaires pour accépter aussi la sortie proposée par HtmlPurifier quand elle divergeait de celle de safehtml.

Topito !

> J'ai réintégré aussi bien que possible le patch via un diff/merge depuis la version du plugin, vu que la source https://gitlab.com/9hozt/html5purfier mentionnée par https://git.spip.net/spip-contrib-extensions/htmlpurifier/commit/defcefbdcedbf9f5ee533bd400523d31bfd04cb5 n'est visiblement pas/plus accessible. > (mais vas-y paye ton diff sur un fichier de 24000 lignes...) huhu, la misère. > A voir si ça nous plait plus d'intégrer https://github.com/xemlock/htmlpurifier-html5 qui est une surcouche du purifier, sans fork donc. ça me semble plus pérenne de prendre cette option :) > J'ai aussi pris le temps de regarder les tests unitaires en défaut suite au changement de lib, puisque le html généré n'est pas le même. Rien de grave, j'ai donc complété les tests unitaires pour accépter aussi la sortie proposée par HtmlPurifier quand elle divergeait de celle de safehtml. Topito !
Owner
// Deprecated PHP 8.1
Deprecated: Return type of HTMLPurifier_PropertyListIterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8360
Deprecated: Return type of HTMLPurifier_StringHash::offsetGet($index) should either be compatible with ArrayObject::offsetGet(mixed $key): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8478

// Deprecated PHP 8.2
Deprecated: Creation of dynamic property HTMLPurifier_Lexer_DOMLex::$_entity_parser is deprecated in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 7814
Deprecated: Creation of dynamic property HTMLPurifier_AttrTransform_NameSync::$idDef is deprecated in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 14339
Deprecated: Creation of dynamic property HTMLPurifier_Lexer_DOMLex::$_entity_parser is deprecated in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 7814
```php // Deprecated PHP 8.1 Deprecated: Return type of HTMLPurifier_PropertyListIterator::accept() should either be compatible with FilterIterator::accept(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8360 Deprecated: Return type of HTMLPurifier_StringHash::offsetGet($index) should either be compatible with ArrayObject::offsetGet(mixed $key): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 8478 // Deprecated PHP 8.2 Deprecated: Creation of dynamic property HTMLPurifier_Lexer_DOMLex::$_entity_parser is deprecated in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 7814 Deprecated: Creation of dynamic property HTMLPurifier_AttrTransform_NameSync::$idDef is deprecated in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 14339 Deprecated: Creation of dynamic property HTMLPurifier_Lexer_DOMLex::$_entity_parser is deprecated in ~/plugins-dist/safehtml/lib/htmlpurifier/HTMLPurifier.standalone.php on line 7814 ```
Owner

Les deprecated semblent corrigées sur le dépot HTML Purifier mais sans nouvelle version (1.14.0 actuellement) : https://github.com/ezyang/htmlpurifier/

Depuis le dépot (cloné localement), on peut générer le fichier standalone ainsi que son répertoire, en lançant php maintenance/generate-standalone.php ce qui (re)crée le fichier et répertoire dans library/

Les deprecated semblent corrigées sur le dépot HTML Purifier mais sans nouvelle version (1.14.0 actuellement) : https://github.com/ezyang/htmlpurifier/ Depuis le dépot (cloné localement), on peut générer le fichier standalone ainsi que son répertoire, en lançant `php maintenance/generate-standalone.php` ce qui (re)crée le fichier et répertoire dans `library/`
Owner

Le diff de 422fc6568b?w=1 (il faut git show -b parce que ça passe de CRLF à LF)

Initialement le fork provenait d’un dépot 9hozt/html5purfier sur Gitlab.com, mais qui n’existe plus.

On trouve une autre adaptation HTML5 https://github.com/xemlock/htmlpurifier-html5/releases également

J’ai commencé à proposer d’utiliser cette librairie qui réduit drastiquement les surcharges réalisées (mais il en reste pour la gestion de data-) dans la branche https://git.spip.net/spip/safehtml/src/branch/refactor_texte_safety_html5

Par ailleurs une autre discussion https://github.com/pkp/pkp-lib/issues/7916 cherche à remplacer HTML_Purifier qui n’est, semble t’il, pas véritablement maintenu, mis à part quelques patchs à la marge de compatibilité.

Iels indiquent en remplacement :

tgalopin qui indique que… il faut utiliser symfony/html-sanitizer aussi maintenant…

Le diff de https://git-mirror.spip.net/spip/safehtml/-/commit/422fc6568ba8717c5e24ab7e163c72780ed1064d?w=1 (il faut `git show -b` parce que ça passe de CRLF à LF) Initialement le fork provenait d’un dépot 9hozt/html5purfier sur Gitlab.com, mais qui n’existe plus. On trouve une autre adaptation HTML5 https://github.com/xemlock/htmlpurifier-html5/releases également J’ai commencé à proposer d’utiliser cette librairie qui réduit drastiquement les surcharges réalisées (mais il en reste pour la gestion de data-) dans la branche https://git.spip.net/spip/safehtml/src/branch/refactor_texte_safety_html5 Par ailleurs une autre discussion https://github.com/pkp/pkp-lib/issues/7916 cherche à remplacer HTML_Purifier qui n’est, semble t’il, pas véritablement maintenu, mis à part quelques patchs à la marge de compatibilité. Iels indiquent en remplacement : - https://github.com/tgalopin/html-sanitizer ou - https://github.com/symfony/html-sanitizer (php 8.1 minimum) tgalopin qui indique que… il faut utiliser symfony/html-sanitizer aussi maintenant…
b_b commented 1 month ago
Owner

Ha ben on causait justement de html-sanitizer dans spip/spip#3926 ^^

Vu que le changement n'a rien d'urgent, il serait bien de prendre notre temps pour tester une variante avec cette lib plutôt que de partir vers une option en fin de vie.

Si on souhaite faire ça pour la 4.2, on ne peut pas aller vers la lib symfony puisque la 4.2 annonce une compat 7.4 => 8.2. Par contre, rien ne nous empêche de basculer vers celle-ci dans une version future qui nécessitera PHP 8.1 mini.

Ha ben on causait justement de html-sanitizer dans https://git.spip.net/spip/spip/issues/3926#issuecomment-35085 ^^ Vu que le changement n'a rien d'urgent, il serait bien de prendre notre temps pour tester une variante avec cette lib plutôt que de partir vers une option en fin de vie. Si on souhaite faire ça pour la 4.2, on ne peut pas aller vers la lib symfony puisque la 4.2 annonce une compat 7.4 => 8.2. Par contre, rien ne nous empêche de basculer vers celle-ci dans une version future qui nécessitera PHP 8.1 mini.
Owner

Voir la PR #4783 qui succède.

Voir la PR #4783 qui succède.
marcimat closed this pull request 1 month ago
b_b deleted branch refactor_texte_safety 1 month ago
This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.