Ajout d'une fonction _requestq() #9

Closed
tofulm wants to merge 1 commits from requestq into master
tofulm commented 2 years ago
Collaborator

qui ajoute à la fonction request() le traitement via la lib HTMLPurifier

qui ajoute à la fonction request() le traitement via la lib HTMLPurifier
tofulm added 1 commit 2 years ago
aabd9b72c0 POC pour l'ajout d'une fonction _requestq()
qui ajoute à la fonction request() le traitement via la lib HTMLPurifier
cerdic commented 2 years ago
Owner

ça me parait totalement hors du scope de bonux. Par ailleurs il y a déjà un plugin htmlpurifier, il serait bien de regrouper plutot que d'avoir plusieurs fois la lib non ?

ça me parait totalement hors du scope de bonux. Par ailleurs il y a déjà un plugin htmlpurifier, il serait bien de regrouper plutot que d'avoir plusieurs fois la lib non ?
tofulm commented 2 years ago
Poster
Collaborator

le plugin htmlpurifier fait la purification à l'affichage, donc plus couteux lors de l'affichage de la page. l'idée de requestq c'est bien de purifier avant l'insertion en bdd.

Je prefère que la validation d'un formulaire soit un peu plus longue mais que l'affichage des pages soit plus rapide.

le plugin htmlpurifier fait la purification à l'affichage, donc plus couteux lors de l'affichage de la page. l'idée de requestq c'est bien de purifier avant l'insertion en bdd. Je prefère que la validation d'un formulaire soit un peu plus longue mais que l'affichage des pages soit plus rapide.
cerdic commented 2 years ago
Owner

Tu peux refaire la PR en séparant d'une part l'ajout de la lib et d'autre part le code/les fonctions que tu ajoutes/propose ?

Là en l'état je peux pas lire le contenu du commit ni avoir d'avis : 395 fichiers ajoutés, et "Certains fichiers n'ont pas été affichés car il y a eu trop de fichiers modifiés dans ce diff"

Par ailleurs je pense que _requestq est pas un nommage adéquat même si je vois l'idée.

Les fonctions suffixées par un q dans l'api mysql font simplement l'échappement des guillemets (le q de quote) sans modification du contenu, alors que là il s'agit de purifier le contenu, donc avec de la suppression potentiellement de balises jugées dangereuse.

Donc il faut un nom plus clair comme _request_purifier().

Enfin, quel est l'intérêt d'encapsuler ça dans un homologue de _request() ?

Il ne sera pas possible de remplacer _request() par un _request_purifier() car pour des raisons évidentes il y a plein de cas où veut absolument le contenu intact.

Et écrire

$a = _request_purifier('a');

plutot que

$a = purifier(_request('a'));

ne change pas grand chose, mais au moins la fonction purifier() est utilisable à d'autres endroits.

Pour finir, peut-être qu'il faut faire évoluer le plugin htmlpurifier pour avoir plusieurs options d'activation comme:

  • niveau 1 : il gère simplement la fonction safehtml avec une lib plus moderne
  • niveau 2 : il filtre tous les affichages (comme c'est le cas actuellement je crois)

Et y ajouter, si pas déjà dispo, ta proposition de fonction.

Mais ça me parait quand même assez contre-productif de disperser les applications et usages de la lib dans différent plugins

Tu peux refaire la PR en séparant d'une part l'ajout de la lib et d'autre part le code/les fonctions que tu ajoutes/propose ? Là en l'état je peux pas lire le contenu du commit ni avoir d'avis : 395 fichiers ajoutés, et "Certains fichiers n'ont pas été affichés car il y a eu trop de fichiers modifiés dans ce diff" Par ailleurs je pense que `_requestq` est pas un nommage adéquat même si je vois l'idée. Les fonctions suffixées par un `q` dans l'api mysql font simplement l'échappement des guillemets (le q de quote) sans modification du contenu, alors que là il s'agit de purifier le contenu, donc avec de la suppression potentiellement de balises jugées dangereuse. Donc il faut un nom plus clair comme `_request_purifier()`. Enfin, quel est l'intérêt d'encapsuler ça dans un homologue de `_request()` ? Il ne sera pas possible de remplacer `_request()` par un `_request_purifier()` car pour des raisons évidentes il y a plein de cas où veut absolument le contenu intact. Et écrire ``` $a = _request_purifier('a'); ``` plutot que ``` $a = purifier(_request('a')); ``` ne change pas grand chose, mais au moins la fonction `purifier()` est utilisable à d'autres endroits. Pour finir, peut-être qu'il faut faire évoluer le plugin htmlpurifier pour avoir plusieurs options d'activation comme: - niveau 1 : il gère simplement la fonction safehtml avec une lib plus moderne - niveau 2 : il filtre tous les affichages (comme c'est le cas actuellement je crois) Et y ajouter, si pas déjà dispo, ta proposition de fonction. Mais ça me parait quand même assez contre-productif de disperser les applications et usages de la lib dans différent plugins
Owner

Mais ça me parait quand même assez contre-productif de disperser les applications et usages de la lib dans différent plugins

plutôt d'accord avec ça pour l'instant

> Mais ça me parait quand même assez contre-productif de disperser les applications et usages de la lib dans différent plugins plutôt d'accord avec ça pour l'instant
Collaborator

Il ne sera pas possible de remplacer _request() par un _request_purifier() car pour des raisons évidentes il y a plein de cas où veut absolument le contenu intact.

heu... mais là il n'est pas du tout question de remplacer _request() mais simplement de proposer une syntaxe rapide et intuitive pour sanitizer les valeurs arrivant d'un formulaire public.
Et l'idée de l'ajout dans bonux c'était simplement de la mettre à disposition facilement sans installation du plugin purifier qui modifie tout un tas d'autres trucs.

> Il ne sera pas possible de remplacer _request() par un _request_purifier() car pour des raisons évidentes il y a plein de cas où veut absolument le contenu intact. heu... mais là il n'est pas du tout question de *remplacer* `_request()` mais simplement de proposer une syntaxe rapide et intuitive pour sanitizer les valeurs arrivant d'un formulaire public. Et l'idée de l'ajout dans bonux c'était simplement de la mettre à disposition facilement sans installation du plugin purifier qui modifie tout un tas d'autres trucs.
tofulm commented 2 years ago
Poster
Collaborator
la fonction _requestq() : https://git.spip.net/spip-contrib-extensions/spip-bonux/src/branch/requestq/spip_bonux_fonctions.php#L189
Collaborator

on ferme cette PR (éventuellement, quand on aura bien rodé de notre côté la fonction requests(), on en fera un plugin à part...)

on ferme cette PR (éventuellement, quand on aura bien rodé de notre côté la fonction `requests()`, on en fera un plugin à part...)
cy.altern closed this pull request 2 years ago
Please reopen this pull request to perform a merge.
You can also view command line instructions.

Step 1:

From your project repository, check out a new branch and test the changes.
git checkout -b requestq master
git pull origin requestq

Step 2:

Merge the changes and update on Gitea.
git checkout master
git merge --no-ff requestq
git push origin master
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: spip-contrib-extensions/spip-bonux#9
Loading…
There is no content yet.