Détecter à la compilation les mauvais nb d'arguments des appels de filtres #160

Closed
JLuc wants to merge 1 commits from JLuc/spip:jluc_issue_4717 into master
JLuc commented 4 weeks ago

Améliore le fix de #4717
Détecte à la compilation les erreurs de nb d'arguments des filtres avec les fonctions d'introspection PHP , ce qui permet un message d'erreur dans la langue de l'internaute avec des référence au source SPIP (au lieu du source PHP compilé que personne n'est sensé connaître).

En l'état, ça détaille le pb dans un log mais ça utilise la chaine zbug_erreur_filtre généraliste pré-existante.
Il me semble qu'on pourrait créer une chaine spécifique à l'erreur nb d'argument...

Améliore le fix de #4717 Détecte à la compilation les erreurs de nb d'arguments des filtres avec les fonctions d'introspection PHP , ce qui permet un message d'erreur dans la langue de l'internaute avec des référence au source SPIP (au lieu du source PHP compilé que personne n'est sensé connaître). En l'état, ça détaille le pb dans un log mais ça utilise la chaine zbug_erreur_filtre généraliste pré-existante. Il me semble qu'on pourrait créer une chaine spécifique à l'erreur nb d'argument...
JLuc added 1 commit 4 weeks ago
e333199c66 Détecter à la compilation les mauvais nb d'arguments des appels de filtres
Owner

Y a plusieurs trucs qui me chagrinnent

  • phraser_arite_incorrecte() devrait retourner un bool, pas un string " " ou "".

  • phraser_arite_incorrecte() ne tient pas compte :

    • d'un éventuel premier paramètre monfiltre(array $Pile) qui est traité dans sandbox
    • d'éventuels func_get_args() dans le code de la fonction

Du coup, je pense que ça va déclarer potentiellement des erreurs qui n'en sont pas inutilement.

Y a plusieurs trucs qui me chagrinnent - phraser_arite_incorrecte() devrait retourner un bool, pas un string " " ou "". - phraser_arite_incorrecte() ne tient pas compte : - d'un éventuel premier paramètre `monfiltre(array $Pile)` qui est traité dans sandbox - d'éventuels `func_get_args()` dans le code de la fonction Du coup, je pense que ça va déclarer potentiellement des erreurs qui n'en sont pas inutilement.
Poster

Le func_get_args ne peut être utilisé qu'à l'intérieur d'une fonction USER et les tests sont moins stricts dans ce cas : on ne vérifie pas l'arité maximale parcequ'effectivement on ne peut pas savoir. Ça ne pose donc pas de problème.
Exemple : le filtre |concat, déclaré sans argument.

Au passage il serait intéressant d'augmenter les capacités d'introspection de SPIP en déclarant explicitement comme variadiques les filtres qui ne sont pas déclarés comme tels mais qui font usage de func_get_args. Ce qui donnerait :
function concat (...$args){ return join('', $args); }
au lieu de l'actuel :
function concat() { $args = func_get_args(); return join('', $args); }
D'ailleurs cette 2eme manière de faire "n'est pas recommandée" depuis l'introduction de la syntaxe "..."
https://www.php.net/manual/fr/functions.arguments.php#functions.variable-arg-list

Sur IRC marcimat

  • pointe sandbox_composer_filtre https://git.spip.net/spip/spip/src/branch/master/ecrire/public/sandbox.php#L60 qui ajoute l'argument $Pile quand il y en a besoin. Effectivement il faut tenir compte de cet argument ajouté en dehors du source SPIP. Ce feature est utilisé pour les filtres |setenv, |set et |sanitize_env dans le core et pour 2 filtres du plugin-dist medias : filtre_medias_modele_document_standard_attributs_dist et filtre_medias_modele_document_standard_classes_dist
  • suggère de ne pas affecter cette vérification au phraser mais au compilateur : peut être dans sandbox_composer_filtre ou à l'appel de ce sandbox.
  • pointe aussi les filtres |image_xx : ils passent dans filtrer('image_xx', texte, ...) puis dans image_filtrer('image_xx', texte, ...) puis dans image_xx(img, ...) (sans le texte, mais avec l'image) donc là on fait des substitutions de variables. Ça change pas le nombre certes… mais potentiellement on pourrait ajouter, enlever…
Le func_get_args ne peut être utilisé qu'à l'intérieur d'une fonction USER et les tests sont moins stricts dans ce cas : on ne vérifie pas l'arité maximale parcequ'effectivement on ne peut pas savoir. Ça ne pose donc pas de problème. Exemple : le filtre |concat, déclaré sans argument. Au passage il serait intéressant d'augmenter les capacités d'introspection de SPIP en déclarant explicitement comme variadiques les filtres qui ne sont pas déclarés comme tels mais qui font usage de func_get_args. Ce qui donnerait : `function concat (...$args){ return join('', $args); }` au lieu de l'actuel : `function concat() { $args = func_get_args(); return join('', $args); }` D'ailleurs cette 2eme manière de faire "n'est pas recommandée" depuis l'introduction de la syntaxe "..." https://www.php.net/manual/fr/functions.arguments.php#functions.variable-arg-list Sur IRC marcimat - pointe sandbox_composer_filtre https://git.spip.net/spip/spip/src/branch/master/ecrire/public/sandbox.php#L60 qui ajoute l'argument $Pile quand il y en a besoin. Effectivement il faut tenir compte de cet argument ajouté en dehors du source SPIP. Ce feature est utilisé pour les filtres `|setenv`, `|set` et `|sanitize_env` dans le core et pour 2 filtres du plugin-dist medias : `filtre_medias_modele_document_standard_attributs_dist` et `filtre_medias_modele_document_standard_classes_dist` - suggère de ne pas affecter cette vérification au phraser mais au compilateur : peut être dans sandbox_composer_filtre ou à l'appel de ce sandbox. - pointe aussi les filtres |image_xx : ils passent dans filtrer('image_xx', texte, ...) puis dans image_filtrer('image_xx', texte, ...) puis dans image_xx(img, ...) (sans le texte, mais avec l'image) donc là on fait des substitutions de variables. Ça change pas le nombre certes… mais potentiellement on pourrait ajouter, enlever…
Owner

+1 pour utiliser les variadic où on peut...

+1 pour utiliser les variadic où on peut...
JLuc commented 5 days ago
Poster

La nouvelle PR prend ces remarques en compte : #170

La nouvelle PR prend ces remarques en compte : https://git.spip.net/spip/spip/pulls/170
JLuc closed this pull request 5 days ago
Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.