Development #38049
ne pas modifier formdata quand il est copié ou picklé
0%
Description
FormData.__getstate__ modifie directement self.__dict__ au lieu de le copier avant.
Fichiers
Révisions associées
Historique
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
- Fichier 0001-formdata-do-not-modify-__dict__-in-__getstate__-3804.patch 0001-formdata-do-not-modify-__dict__-in-__getstate__-3804.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Frédéric Péters il y a plus de 4 ans
Ça pose quel problème, qui peut être exposé dans un test ?
Mis à jour par Benjamin Dauvergne il y a plus de 4 ans
Ça ma causé des soucis sur #33186 (initialisation d'un brouillon), quand j'ai voulu ajouté le support "pickle-lazy" il fallait à la fois ajouter la nouvelle variable linked dans FormData.get_substitution_variables() mais aussi la supprimer1 dans FormData.get_static_substitution_variables() sinon linked était copié par deecopy() à un moment et le formdata transient se retrouver sans ._formdef.
On peut voir les traces du à ce souci sur jenkins2.
C'est pas en soi un bug existant puisque tout est fait pour l'éviter actuellement (je ne sais pas si c'est ça qui a mené au nettoyage de lazy avec des del), mais ça évitera à un autre d'avoir à s'arracher les cheveux plus tard.
2 https://jenkins.entrouvert.org/job/wcs-wip/job/wip%252F33186-Initialisation-d-un-brouillon/3/
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
branche rebasée, pour moi le problème est toujours là sous-jacent (clairement faire un .copy() ne va rien casser).
Mis à jour par Frédéric Péters il y a environ 4 ans
- Statut changé de Solution proposée à Solution validée
Pour mémoire j'ai du mal à valider du changement de code juste sur un hypothétique futur sur lequel je devrais sans doute prendre plus de temps pour réfléchir. Mais j'ai un attrait pour avoir du code uniforme et donc l'argument comme quoi c'est déjà fait ainsi ailleurs, il marcherait sans me demander d'effort :
# don't pickle _formata cache
def __getstate__(self):
odict = self.__dict__.copy()
if '_formdata' in odict:
del odict['_formdata']
if '_display_parts' in odict:
del odict['_display_parts']
return odict
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
Frédéric Péters a écrit :
Pour mémoire j'ai du mal à valider du changement de code juste sur un hypothétique futur sur lequel je devrais sans doute prendre plus de temps pour réfléchir. Mais j'ai un attrait pour avoir du code uniforme et donc l'argument comme quoi c'est déjà fait ainsi ailleurs, il marcherait sans me demander d'effort :
[...]
J'en savais foutre rien que c'était déjà comme ça sur une autre classe, c'est une découverte indépendante.
Mis à jour par Benjamin Dauvergne il y a environ 4 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 6a63b81928f28211eaab824840798a5954b374b2
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Thu Nov 28 12:29:41 2019 +0100
formdata: do not modify __dict__ in __getstate__ (#38049)
It suppress the _formdef cache, which can break things with transient
formdata created from the FormData class instead of a FormDef.data_class().
Ideally transient FormData should be never copied as the copy is lacking
a _formdef attribute and _names is not initialised so
FormData.get_formdef() will never work.Mis à jour par Frédéric Péters il y a environ 4 ans
- Statut changé de Résolu (à déployer) à Solution déployée
formdata: do not modify dict in getstate (#38049)
It suppress the _formdef cache, which can break things with transient
formdata created from the FormData class instead of a FormDef.data_class().
Ideally transient FormData should be never copied as the copy is lacking
a _formdef attribute and _names is not initialised so
FormData.get_formdef() will never work.