Bug #15379

crash suite à écrasement d'un formulaire, crash lié au format SQL

Ajouté par Thomas Noël il y a presque 2 ans. Mis à jour il y a 4 jours.

Statut:NouveauDébut:10 mar. 2017
Priorité:NormalEchéance:
Assigné à:-% réalisé:

0%

Catégorie:-
Version cible:-
Patch proposed:Non

Description

LINE 1: ...ULL, submission_context = NULL, f23 = NULL, f22 = '2010-01-2...
                                                             ^
HINT:  You will need to rewrite or cast the expression.
)
To: admin+cch-omonville@entrouvert.com

Exception:
  type = '<class 'psycopg2.ProgrammingError'>', value = 'column "f22" is of type text[] but expression is of type timestamp without
time zone
LINE 1: ...ULL, submission_context = NULL, f23 = NULL, f22 = '2010-01-2...
                                                             ^
HINT:  You will need to rewrite or cast the expression.
'

Stack trace (most recent call first):
  File "/usr/lib/python2.7/dist-packages/wcs/sql.py", line 1194, in store
  1192                                        self._table_name,
  1193                                        ', '.join(['%s = %%(%s)s' % (x,x) for x in column_names]))
> 1194             cur.execute(sql_statement, sql_dict)
  1195             if cur.fetchone() is None:
  1196                 column_names = sql_dict.keys()

Le champ 22 du formulaire est de type date, mais dans le SQL on a un text[]

Le formulaire a été "overwrité" :

78.247.170.27 - - [08/Mar/2017:09:04:23 +0100] "POST /backoffice/forms/2/overwrite HTTP/1.1" 302 109 "https://omonville-la-rogue.test.entrouvert.org/backoffice/forms/2/overwrite" "Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0" 

Je pose ce ticket sans trop savoir ce qu'il faut faire ; mais je dirais que lors d'un overwrite on devrait peut-être vérifier que le type des champs de même id correspond (et refuser l'overwrite) ?

Historique

#1 Mis à jour par Thomas Noël il y a 5 jours

Le cas se produit quand même régulièrement, les gens cochent sur "Overwrite nevertheless" et hop...

        get_request().method = 'GET'
        get_request().form = {}
        form = Form(enctype='multipart/form-data', use_tokens=False)
        if different_type_fields:
            form.widgets.append(HtmlWidget('<div class="errornotice"><p>%s</p></div>' % _(
                'The form has incompatible fields, it may cause data corruption and bugs.')))
            form.add(CheckboxWidget, 'force', title=_('Overwrite nevertheless'))
        else:
            form.add_hidden('force', 'ok')
        form.add_hidden('new_formdef', ET.tostring(new_formdef.export_to_xml(include_id=True)))
        form.add_submit('submit', _('Submit'))
        form.add_submit('cancel', _('Cancel'))
        r += form.render()

Il faut être plus violent, et ne plus proposer l'overwrite en cas de different_type_fields.

#3 Mis à jour par Frédéric Péters il y a 5 jours

régulièrement

Par rapport à combien de fois où forcer les choses "marche" ? (grep POST.*overwrite sur les access.log, ça pourrait nous donner une indication ?)

#4 Mis à jour par Thomas Noël il y a 5 jours

Frédéric Péters a écrit :

régulièrement

Par rapport à combien de fois où forcer les choses "marche" ? (grep POST.*overwrite sur les access.log, ça pourrait nous donner une indication ?)

Ok, j'ai exagéré, vous me connaissez. Régulièrement = 2 fois cette semaine, dont une fois en prod avec du jeu de backup pas funky, et ça me suffit pour dire qu'il faut interdire cette manip et c'est tout.

#5 Mis à jour par Benjamin Dauvergne il y a 4 jours

Et générer des id de field uniques plutôt ?

In [5]: 'f' + base64.urlsafe_b64encode(os.urandom(8)).strip('=')
Out[5]: 'f5FR91ww4FFs'

Aucune chance qu'en cas de fork d'un formulaire un ré-import crée une collision.

#6 Mis à jour par Benjamin Dauvergne il y a 4 jours

Plutôt de l'encodage base32 pour éviter les problèmes de casse (je ne sais pas si on quote ou pas les noms de colonne dans la partie SQL, mais le mieux c'est d'éviter le problème).

#7 Mis à jour par Thomas Noël il y a 4 jours

En fait on pourrait juste prendre des nouveaux fNNN pour ces champs en conflit.

Mais en fait, ces conflits de champs, ils traduisent une mauvaise procédure de travail (normalement on bosse sur une copie du formulaire en prod, et il est impossible de changer le type d'un champ, donc ça peut pas arriver). C'est pour ça que je pense vraiment juste interdire l'overwrite, et que la personne doive reprendre son travail correctement.

#8 Mis à jour par Benjamin Dauvergne il y a 4 jours

Thomas Noël a écrit :

En fait on pourrait juste prendre des nouveaux fNNN pour ces champs en conflit.

Mais en fait, ces conflits de champs, ils traduisent une mauvaise procédure de travail (normalement on bosse sur une copie du formulaire en prod, et il est impossible de changer le type d'un champ, donc ça peut pas arriver). C'est pour ça que je pense vraiment juste interdire l'overwrite, et que la personne doive reprendre son travail correctement.

Sauf que c'est inexplicable aux gens (vous avez forké, et donc on a généré des id identiques pour des champs différents découlant d'un travail sur deux plate-formes différentes). Si on interdit de forcer l'overwrite sans rien faire de plus, on aura des tickets de support pour "j'arrive pas à mettre à jour" en pagaille.
Si on force l'overwrite et qu'on génère des id globalement uniques, on évite les problèmes et on permet aux gens de travailler (et de faire vraisemblablement de la merde quand ils verront qu'un champ x créé des deux cotés ne reprend pas les données du champ en prod, mais ça c'est le boulot de l'écran de dire les données du champs X vont être perdu, are you ok ? vous devriez partir d'un export d'un formulaire avant de le modifier pour éviter ces problèmes).

#9 Mis à jour par Thomas Noël il y a 4 jours

mais ça c'est le boulot de l'écran de dire les données du champs X vont être perdu, are you ok ?

C'est déjà le cas on affiche déjà des warning, mais "les gens" (dont nous) cliquent sur "oui oui vazy je m'en fous", et boum. Moi j'ai vraiment plus envie, jamais, d'avoir à faire le pompier en prod sur ce genre de soucis, aussi petit soit-il (et parce qu'en plus, il ne peut être visible que x jours après).

Si on met pas le bouton, alors les gens sont bien obligés de lire ce qui est écrit, et on peut éventuellement rappeler la procédure (repartir du formulaire actuel).

En tout cas moi, je préfère ça, et si on le fait pas la prochaine fois c'est toi qui remonte les backups :)

Formats disponibles : Atom PDF