Ajout d'une fonction _requestq() #9

Closed
tofulm wants to merge 1 commits from requestq into master
tofulm commented 4 months ago

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 4 months ago
aabd9b72c0 POC pour l'ajout d'une fonction _requestq()
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 ?
Poster

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.
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

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

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.
Poster
la fonction _requestq() : https://git.spip.net/spip-contrib-extensions/spip-bonux/src/branch/requestq/spip_bonux_fonctions.php#L189

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 months ago
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.