[spip-dev] Couche d'abstraction SQL / Formidable

Dev
1

Je reporte ici une conversation de la zone qui a probablement sa place ici.

je suis en train d'étudier plusieurs options pour régler un problème de concurrence de requêtes qui génère très souvent des doublons dans des réponses à des formulaires à réponse unique sur Formidable.

J'explique rapidement le contexte, mais ça pourrait faire l'objet d'un autre thread dans la Zone. Une personne qui valide plusieurs fois un formulaire Formidable sans en attendre le résultat peut très bien créer autant de réponses dans la table spip_formulaires_reponses, qu'il a cliqué sans attendre la réponse.

J'ai proposé à RastaPopoulos, en MP, de créer un nouveau champ contenant un hash unique (une clé SQL) dans la table spip_formulaires_reponses. Ça ferait échouer un deuxième INSERT avec le même hash et empêcherait la duplication des réponses.

L'autre solution, qui m'amène à écrire ce courriel, est de régler le problème via des transactions SQL. On ouvre une transaction, on fait notre insert, on referme. Pas de possibilité de faire d'autres opérations en base tant que le COMMIT n'est pas fait.

Je suis tombé sur cette page sur le web :
http://code.spip.net/autodoc/tree/ecrire/base/abstract_sql.php.html#function_sql_demarrer_transaction

où il y a bien deux fonctions sql_demarrer_transaction() et sql_terminer_transaction() qui ont l'air de faire ça, même si elles ne sont pas documentées. En fouillant les sources, les fonctions sont disponibles pour SQLite, mais pas pour les bases MySQL, qui sont également sensées gérer les transactions.

Est-ce que c'est pour une bonne raison que les fonctions de transaction ne sont pas implémentées sous MySQL ? Quelqu'un s'y serait-il déjà frotté et aurait-il battu en retraite ?

En admettant que je modifie Formidable pour utiliser ces fonctions, et que je les implémente pour SPIP, Formidable deviendrait dépendant de la toute dernière version de SPIP ce qui risque de casser la compatibilité sur beaucoup de site.

Est-ce que vous pensez que le problème du "clic frénétique" serait quelque chose à régler directement au niveau de SPIP ?
Est-ce qu'il n'y aurait pas de limitations à mettre directement au niveau du mécanisme CVT par exemple ?

J'avais aussi parlé en MP d'une solution Javascript, à savoir un clic sur un bouton "Envoyer" invaliderait le bouton en lui-même (disabled). Ça limiterait les fausses manipes. Par contre, le contrôle étant effectué côté client, ça ne mettrait pas à l'abri des malversations.

Ci-dessous, la réponse de RastaPopoulos (désolé si la structure de ce courriel est un poil brouillonne).

2

Hello,

bonne question et bon sujet de reflexion !
Historiquement ce problème là est déjà géré par SPIP pour les forums (dans le plugins-dist/forum donc)

En pratique un fichier lock est crée lorsqu'on charge le formulaire, avec un identifiant/alea unique injecté en <input hidden />
Ensuite lors du POST, si le fichier lock est encore là, tout va bien : on le supprime et on traite le formulaire normalement.
Si le fichier lock n'est plus là, c'est un double hit, on refuse le traitement.

Donc il me semble qu'il serait en effet très pertinent d'implémenter une solution générique aux CVT basée sur ce principe là, en la mettant au propre pour qu'elle s'insère automatiquement via les pipelines concernés, sans avoir rien à modifier dans les formulaires existant.
Cela peut être développé/mis au point dans un plugin, et une fois stabilisé/débuggué intégré au core car c'est d'un intérêt public certain.
Cela permettra aussi de nettoyer un peu encore le code du plugin forums en en enlevant ce truc spécifique.

Cédric

Camille Sauvage a écrit:

3

En pratique un fichier lock est crée lorsqu'on charge le formulaire,
avec un identifiant/alea unique injecté en <input hidden />
Ensuite lors du POST, si le fichier lock est encore là, tout va bien :
on le supprime et on traite le formulaire normalement.
Si le fichier lock n'est plus là, c'est un double hit, on refuse le
traitement.

Nous avons déjà parlé de ça avec RastaPopoulos. Le coup du champ "hidden" place la "confiance" dans le client pour fournir le nom du fichier .lock au serveur.

Dans la dernière version de Formidable, nous avons abandonné l'usage d'un hidden "deja_enregistre_ID_FORMULAIRES_REPONSE" qui disait au serveur de quel id_formulaires_reponses se servir. Ça permettait même à un client d'accéder aux réponses d'un autre utilisateur.

Est-ce qu'on a pas moyen de garder un hash, ou aléa dans un formulaire CVT, au moins commun aux étapes C et surtout V qui nous permettrait de nous affranchir du hidden ?

4

En regardant un peu mieux la structure de SPIP (je maîtrise assez bien les plugins mais pas trop le core), je voudrais proposer la formule suivante :
un plugin qui va se servir des trois pipelines appelés pendant les opérations CVT.

1/ À l'appel de "formulaire_charger", on crée le fichier "lck" et on le passe en "hidden" ou par un autre moyen pour le passer à "verifier" sans passer par le client (?!).

2/ À l'appel de "formulaire_verifier", on déclenche une erreur de clic frénétique si le .lck a disparu.

3/ À l'appel de "formulaire_traiter", on supprime le fichier .lck

Ça paraît tellement simple que ça m'en semble un peu naïf.
À part le problème du nom de fichier à se repasser entre les trois fonctions, je ne vois pas trop d'autres subtilités à traiter.

Qu'en dites-vous ?

5

Camille Sauvage a écrit :

En pratique un fichier lock est crée lorsqu'on charge le formulaire,
avec un identifiant/alea unique injecté en <input hidden />
Ensuite lors du POST, si le fichier lock est encore là, tout va bien :
on le supprime et on traite le formulaire normalement.
Si le fichier lock n'est plus là, c'est un double hit, on refuse le
traitement.

Nous avons déjà parlé de ça avec RastaPopoulos. Le coup du champ
"hidden" place la "confiance" dans le client pour fournir le nom du
fichier .lock au serveur.

Dans une moindre mesure car si ici le client essaye de jouer avec le lck, tout ce qu'il gagnera c'est que son POST sera refusé, car si il ne fournit pas le lck qui correspond au fichier qui a été posé lors de charger() il ne pourra rien faire.

Dans la dernière version de Formidable, nous avons abandonné l'usage
d'un hidden "deja_enregistre_ID_FORMULAIRES_REPONSE" qui disait au
serveur de quel id_formulaires_reponses se servir. Ça permettait même à
un client d'accéder aux réponses d'un autre utilisateur.

Oui justement, ici on l'utilise différement et sans divulguer aucune information du serveur ni de la base.

Est-ce qu'on a pas moyen de garder un hash, ou aléa dans un formulaire
CVT, au moins commun aux étapes C et surtout V qui nous permettrait de
nous affranchir du hidden ?

Non, puisque ce qu'on veut c'est bien se prémunir d'un double POST. Pour ça il faut que l'on puisse verifier que ce qui a été posté est légitime ou doublon, donc forcément avoir une trace dans le POST.

En regardant un peu mieux la structure de SPIP (je maîtrise assez bien les plugins mais pas trop le core), je voudrais proposer la formule suivante :
un plugin qui va se servir des trois pipelines appelés pendant les opérations CVT.

1/ À l'appel de "formulaire_charger", on crée le fichier "lck" et on le passe en "hidden" ou par un autre moyen pour le passer à "verifier" sans passer par le client (?!).

Tout à fait, et on le passe en hidden, c'est indispensable !
Il faut utiliser la fonction du plugin forums, mais en en profitant pour stocker les .lck dans un dossier spéficique facile à purger, et peut-être gérer un peu mieux le cas où l'on génère plein de lck à cause des bots.

2/ À l'appel de "formulaire_verifier", on déclenche une erreur de clic frénétique si le .lck a disparu.

Dans le pipeline, il faut passer en dernier, et si il n'y a aucune erreur de saisie, on regarde le .lck et on fait une erreur "double post" en effet si le .lck n'est plus là.
Mais cela pose la question de ce qu'on fait quand on reaffiche le formulaire et qu'on repasse dans charger() : on génère un nouveau lck qui permettra à nouveau de poster ?

3/ À l'appel de "formulaire_traiter", on supprime le fichier .lck

C'est trop tard, car le pipeline traiter va être appelé en dernier lui aussi, et du coup ça laisse le temps d'avoir 2 hits concurrents.
Il faut supprimer le .lck dans le verifier(), au dernier moment d'où la nécessité de passer en dernier dans ce pipeline, pour empecher tout autre POST de pouvoir passer dans le traiter()

Ça paraît tellement simple que ça m'en semble un peu naïf.

Le diable réside dans les détails, as usual.

À part le problème du nom de fichier à se repasser entre les trois fonctions, je ne vois pas trop d'autres subtilités à traiter.

Il n'y a rien à passer, cela repose sur le POST, comme dit plus haut

Qu'en dites-vous ?

gogogo

Cédric

6

Salut Cédric, salut à tutti,

Tout à fait, et on le passe en hidden, c'est indispensable !

Bien compris, je suis également convaincu.

Il faut utiliser la fonction du plugin forums, mais en en profitant pour
stocker les .lck dans un dossier spéficique facile à purger, et
peut-être gérer un peu mieux le cas où l'on génère plein de lck à cause
des bots.

On ne pourrait pas imaginer que plutôt que virer tous ceux ayant une durée de vie supérieure à 4h, on en garde toujours "un certain nombre" et on supprime les surnuméraires par ordre de vieillesse (ne surtout pas imaginer ce principe dans la vraie vie). On est ainsi certains de ne pas encombrer le serveur.
Ça serait intéressant d'avoir des stats moyennes du nombre de fichier .lck qui vivent dans un écosystème plutôt participatif.

2/ À l'appel de "formulaire_verifier", on déclenche une erreur de clic
frénétique si le .lck a disparu.

Mais cela pose la question de ce qu'on fait quand on reaffiche le
formulaire et qu'on repasse dans charger() : on génère un nouveau lck
qui permettra à nouveau de poster ?

Bin oui, je ne vois pas ce que ça peut gêner. Le client veut reposter, c'est son droit, mais au moins cette fois, c'est légitime.

3/ À l'appel de "formulaire_traiter", on supprime le fichier .lck

C'est trop tard, car le pipeline traiter va être appelé en dernier lui
aussi, et du coup ça laisse le temps d'avoir 2 hits concurrents.
Il faut supprimer le .lck dans le verifier(), au dernier moment d'où la
nécessité de passer en dernier dans ce pipeline, pour empecher tout
autre POST de pouvoir passer dans le traiter()

Si je comprends bien, verifier() vérifie l'existence du .lck, le supprime et rend la main sans erreur à traiter(). Et en l'absence du .lck, traiter() ne sera jamais appelé. C'est bien ça ?

Ça paraît tellement simple que ça m'en semble un peu naïf.

Le diable réside dans les détails, as usual.

:slight_smile: c'est aussi un peu ce que je me disais.

Qu'en dites-vous ?

gogogo

Modulo le fait que je pars en vacances ce soir pour 3 semaines. Mais je veux bien m'en charger s'il n'y a pas d'urgence.
Mais les choses sont claires et ça devrait aller vite.

Vu que je me lance dans un plugin pas dist, on devrait peut-être plus continuer dans [spip-zone] non ?