Verification slug et séparateurs #9

Open
opened 2 years ago by Eric · 9 comments
Eric commented 2 years ago
Owner

Si je saisis un username jojo-le-merou ou jojo_le-merou, la verification slug me retourne une erreur même si j'utilise l'option 'separateur' => '_-' ce qui me parait contraire à l'explication de la fonction.

En fait, la première regexp qui vérifie le format:

$pattern_sep = (strlen($separateurs) > 1 ? '[' . preg_quote($separateurs) . ']' : $separateur);
$is_slug = preg_match("/^[a-z0-9]+(?:${pattern_sep}[a-z0-9]+)*\$/", $valeur);

me parait incorrecte car il faudrait que le $pattern_sep soit inclus dans la liste entre crochets. De plus pourquoi ne pas permettre de commencer par un séparateur et donc réduire la regexp à une simple liste de caractères ?

Sinon, dans la la suite on reteste pour la normalisation sans se préoccuper du résultat du premier test. Et comme là on prend que le premier séparateur, je ne vois pas comment ça peut fonctionner.

Après, je n'ai peut-être pas compris la fonction mais elle me parait tout de même bancale non ?

Si je saisis un username `jojo-le-merou` ou `jojo_le-merou`, la verification slug me retourne une erreur même si j'utilise l'option `'separateur' => '_-'` ce qui me parait contraire à l'explication de la fonction. En fait, la première regexp qui vérifie le format: ```php $pattern_sep = (strlen($separateurs) > 1 ? '[' . preg_quote($separateurs) . ']' : $separateur); $is_slug = preg_match("/^[a-z0-9]+(?:${pattern_sep}[a-z0-9]+)*\$/", $valeur); ``` me parait incorrecte car il faudrait que le $pattern_sep soit inclus dans la liste entre crochets. De plus pourquoi ne pas permettre de commencer par un séparateur et donc réduire la regexp à une simple liste de caractères ? Sinon, dans la la suite on reteste pour la normalisation sans se préoccuper du résultat du premier test. Et comme là on prend que le premier séparateur, je ne vois pas comment ça peut fonctionner. Après, je n'ai peut-être pas compris la fonction mais elle me parait tout de même bancale non ?
Collaborator

ping @tcharlss qui avait codé

ping @tcharlss qui avait codé
Owner

Je ne reproduis pas, aucune erreur avec jojo_le-merou + 'separateur' => '_-'

En fait, la première regexp qui vérifie le format [...] me parait incorrecte car il faudrait que le $pattern_sep soit inclus dans la liste entre crochets. De plus pourquoi ne pas permettre de commencer par un séparateur et donc réduire la regexp à une simple liste de caractères ?

C'est voulu, cette vérif s'aligne avec le format produit par identifiant_slug() : pas de séparateur répété, ni situé au début ou à la fin : __jojo---le___merou--jojo_le_merou.

Dans ce cadre, une simple liste de charactères ne suffit pas, il y a un ordre et une structure précise.

Sinon, dans la la suite on reteste pour la normalisation sans se préoccuper du résultat du premier test. Et comme là on prend que le premier séparateur, je ne vois pas comment ça peut fonctionner.

La 2ème vérif n'est qu'une précaution pour ne pas lancer la normalisation en vain, pas de lien avec la 1ère vérif.

Je ne reproduis pas, aucune erreur avec `jojo_le-merou` + `'separateur' => '_-'` > En fait, la première regexp qui vérifie le format \[...] me parait incorrecte car il faudrait que le $pattern_sep soit inclus dans la liste entre crochets. De plus pourquoi ne pas permettre de commencer par un séparateur et donc réduire la regexp à une simple liste de caractères ? C'est voulu, cette vérif s'aligne avec le format produit par `identifiant_slug()` : pas de séparateur répété, ni situé au début ou à la fin : `__jojo---le___merou--` → `jojo_le_merou`. Dans ce cadre, une simple liste de charactères ne suffit pas, il y a un ordre et une structure précise. > Sinon, dans la la suite on reteste pour la normalisation sans se préoccuper du résultat du premier test. Et comme là on prend que le premier séparateur, je ne vois pas comment ça peut fonctionner. La 2ème vérif n'est qu'une précaution pour ne pas lancer la normalisation en vain, pas de lien avec la 1ère vérif.
Owner

suite : après on peut rendre la vérif plus tolérante, mais ça ferait une dichotomie entre ce que la fonction de normalisation considère comme un slug et la vérif.

suite : après on peut rendre la vérif plus tolérante, mais ça ferait une dichotomie entre ce que la fonction de normalisation considère comme un slug et la vérif.
Poster
Owner

Je ne reproduis pas, aucune erreur avec jojo_le-merou + 'separateur' => '_-'

Euh, bizarre voilà ce que j'obtiens l'image ci-dessous.
Le code d'appel est:

	} elseif ($erreur = $verifier(_request('username'), 'slug', array('normaliser_suggerer' => true, 'separateur' => '_-'))) {
			$erreurs['username'] = $erreur;

Et c'est exact que maintenant que je suis mieux réveillé je ne reproduis plus.
Pourtant hier j'ai essayé maintes fois sans succès.

Après, si le format est bon pourquoi faut-il le slugifier ?
Y a quand même un truc que je ne pige pas dans la logique de la vérification.

> Je ne reproduis pas, aucune erreur avec `jojo_le-merou` + `'separateur' => '_-'` Euh, bizarre voilà ce que j'obtiens l'image ci-dessous. Le code d'appel est: ```php } elseif ($erreur = $verifier(_request('username'), 'slug', array('normaliser_suggerer' => true, 'separateur' => '_-'))) { $erreurs['username'] = $erreur; ``` Et c'est exact que maintenant que je suis mieux réveillé je ne reproduis plus. Pourtant hier j'ai essayé maintes fois sans succès. Après, si le format est bon pourquoi faut-il le slugifier ? Y a quand même un truc que je ne pige pas dans la logique de la vérification.
Owner

Après, si le format est bon pourquoi faut-il le slugifier ?

Ben tu utilises l'option « normaliser_suggerer », il faut bien avoir quelque chose à suggérer.

Avec les options de ton exemple et jojo-le_merrou ou jojo-le-merrou, j'obtiens le message complet qui comprend la suggestion, pourquoi tu n'as pas ça ?

Uniquement des caractères alphanumériques en minuscules ou « _- ».
Essayez « jojo_le_merou ».'

En fait il y a un point qui rend les choses sans doute un peu ambigües dans le code : la vérif est un peu plus tolérante que la fonction de normalisation, puisqu'elle accepte plusieurs séparateurs dans un même slug, alors que la normalisation ne le permet pas.
C'est pour ça qu'il y a 2 tests différents pour ces 2 étapes.

Donc à partir de là :

  1. soit on est moins laxistes dans la vérif et on accepte strictement le format reconnu par identifier_slug (1 seul séparateur possible). Ce qui simplifierait le code et la compréhension de la vérif.
  2. soit on ouvre les vannes et on accepte plus de choses (plusieurs séparateurs, possiblement au début ou à la fin, etc.). Mais dans ce cas là il faut adapter les messages d'erreurs et les commentaires.
> Après, si le format est bon pourquoi faut-il le slugifier ? Ben tu utilises l'option « normaliser_suggerer », il faut bien avoir quelque chose à suggérer. Avec les options de ton exemple et `jojo-le_merrou` ou `jojo-le-merrou`, j'obtiens le message complet qui comprend la suggestion, pourquoi tu n'as pas ça ? > Uniquement des caractères alphanumériques en minuscules ou « _- ». Essayez « jojo_le_merou ».' En fait il y a un point qui rend les choses sans doute un peu ambigües dans le code : la vérif est un peu plus tolérante que la fonction de normalisation, puisqu'elle accepte plusieurs séparateurs dans un même slug, alors que la normalisation ne le permet pas. C'est pour ça qu'il y a 2 tests différents pour ces 2 étapes. Donc à partir de là : 1. soit on est moins laxistes dans la vérif et on accepte strictement le format reconnu par identifier_slug (1 seul séparateur possible). Ce qui simplifierait le code et la compréhension de la vérif. 2. soit on ouvre les vannes et on accepte plus de choses (plusieurs séparateurs, possiblement au début ou à la fin, etc.). Mais dans ce cas là il faut adapter les messages d'erreurs et les commentaires.
Poster
Owner

En fait ce qui est perturbant c'est le fait de pouvoir passer plusieurs séparateurs dont un seul est le "bon" et de slugifier sur ce seul séparateur.
La question est à quoi servent les autres séparateurs ? D'ailleurs en quoi sont-ils des séparateurs ?

En fait ce qui est perturbant c'est le fait de pouvoir passer plusieurs séparateurs dont un seul est le "bon" et de slugifier sur ce seul séparateur. La question est à quoi servent les autres séparateurs ? D'ailleurs en quoi sont-ils des séparateurs ?
Owner

Oui l'idée était d'être un peu plus « sympa » en acceptant jojo-le_merrou quand on veux juste faire une vérif, sans normaliser ou suggérer.
Mais ça complique et rend les choses pas claires quand on combine vérif + normalisation.

Donc je suggère de faire le 1) et de virer la possibilité d'avoir plusieurs séparateurs différents. Ton avis ?

Et tu as vu pourquoi tu n'obtiens pas le message d'erreur complet, qui est censé inclure la suggestion ?

Oui l'idée était d'être un peu plus « sympa » en acceptant `jojo-le_merrou` quand on veux juste faire une vérif, sans normaliser ou suggérer. Mais ça complique et rend les choses pas claires quand on combine vérif + normalisation. Donc je suggère de faire le 1) et de virer la possibilité d'avoir plusieurs séparateurs différents. Ton avis ? Et tu as vu pourquoi tu n'obtiens pas le message d'erreur complet, qui est censé inclure la suggestion ?
Poster
Owner

En fait, oui, je trouve la logique biscornue.
Je pense qu'il ne devrait y avoir qu'un seul séparateur dans l'option 'separateur' qui est écrite sans S d'ailleurs.

Après, ce que je trouve très restrictif c'est le [a-z0-9].
Et là peut-être que l'on pourrait avoir une nouvelle option pour rajouter des caractères, c'est à dire ceux qui étaient dans le séparateur.
Mais de toute façon, il faudrait toujours tester la même regexp qui renvoie conforme ou pas, et ensuite faire la normalisation si demandé.

Tu vois le truc ?

En fait, oui, je trouve la logique biscornue. Je pense qu'il ne devrait y avoir qu'un seul séparateur dans l'option 'separateur' qui est écrite sans S d'ailleurs. Après, ce que je trouve très restrictif c'est le `[a-z0-9]`. Et là peut-être que l'on pourrait avoir une nouvelle option pour rajouter des caractères, c'est à dire ceux qui étaient dans le séparateur. Mais de toute façon, il faudrait toujours tester la même regexp qui renvoie conforme ou pas, et ensuite faire la normalisation si demandé. Tu vois le truc ?
Owner

Bon on peut discuter du code, mais en attendant je ne reproduis toujours pas ce que tu décris.
Tu peux me donner quelque chose de reproductible ?
Et tu peux me repréciser ce que tu cherches à obtenir au final ?

Tu vois le truc ?

Oui et non.

Oui pour s'aligner exactement sur la définition d'un slug donnée par identifiant_slug(), avec donc un seul type de séparateur possible : « que des caracteres alphanumeriques et un separateur »
https://git.spip.net/spip/spip/src/branch/master/ecrire/inc/filtres.php#L5172

Et là peut-être que l'on pourrait avoir une nouvelle option pour rajouter des caractères, c'est à dire ceux qui étaient dans le séparateur.

Mais par contre je vois pas trop l'idée là : si on acceptait d'autres caractères dans la liste, ça contredirait la définition.
Dans cette définition, jojo-le_merrou est invalide : pas le droit d'avoir plusieurs séparateurs.
Et en plus ça laisserait passer les répétitions, ce qui serait encore pire : jojo---le___merou.
D'ailleurs ça n'a même pas à être une liste de caractères acceptés, ça pourrait être remplacé par le token \w.


Une autre option que j'envisageais au départ, c'était de tout simplifier en ne faisant pas de regex du tout : se contenter de tester la valeur avec le résultat de identifiant_slug().
Si c'est différent, il ne s'agit donc pas un slug, et voilà.
Mais faire une regex laisse la possibilité de donner des détails plus précis sur l'erreur si on veut, et permettait d'être un peu plus tolérant, donc j'ai fait comme ça pour l'instant.

Bon on peut discuter du code, mais en attendant je ne reproduis toujours pas ce que tu décris. Tu peux me donner quelque chose de reproductible ? Et tu peux me repréciser ce que tu cherches à obtenir au final ? > Tu vois le truc ? Oui et non. Oui pour s'aligner exactement sur la définition d'un slug donnée par identifiant_slug(), avec donc un seul type de séparateur possible : « que des caracteres alphanumeriques et un separateur » https://git.spip.net/spip/spip/src/branch/master/ecrire/inc/filtres.php#L5172 > Et là peut-être que l'on pourrait avoir une nouvelle option pour rajouter des caractères, c'est à dire ceux qui étaient dans le séparateur. Mais par contre je vois pas trop l'idée là : si on acceptait d'autres caractères dans la liste, ça contredirait la définition. Dans cette définition, `jojo-le_merrou` est invalide : pas le droit d'avoir plusieurs séparateurs. Et en plus ça laisserait passer les répétitions, ce qui serait encore pire : `jojo---le___merou`. D'ailleurs ça n'a même pas à être une liste de caractères acceptés, ça pourrait être remplacé par le token `\w`. ---- Une autre option que j'envisageais au départ, c'était de tout simplifier en ne faisant pas de regex du tout : se contenter de tester la valeur avec le résultat de identifiant_slug(). Si c'est différent, il ne s'agit donc pas un slug, et voilà. Mais faire une regex laisse la possibilité de donner des détails plus précis sur l'erreur si on veut, et permettait d'être un peu plus tolérant, donc j'ai fait comme ça pour l'instant.
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.