Development #43789
interdire les slug qui ressemblent à des id (numeros)
0%
Description
On a ce code :
class SlotStatus(APIView):
permission_classes = (permissions.IsAuthenticated,)
def get_object(self, event_identifier):
try:
return Event.objects.get(slug=event_identifier)
except Event.DoesNotExist:
try:
# legacy access by event id
return Event.objects.get(pk=int(event_identifier))
except (ValueError, Event.DoesNotExist):
raise Http404()
<pre>
En imaginant la création d'un event avec un slug "6", il va prendre le dessus sur l'event id 6, qui devient inaccessible.
Ma conclusion : il faut interdire les slug numériques (interdire au plus bas niveau possible, par exemple lors du save(), pour que ça crashe aussi lors d'un import).
Fichiers
Révisions associées
Historique
Mis à jour par Valentin Deniaud il y a presque 4 ans
- Fichier 0001-agendas-forbid-numerical-slug-for-events-43789.patch 0001-agendas-forbid-numerical-slug-for-events-43789.patch ajouté
- Tracker changé de Bug à Development
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
J'ai vérifié en prod et en recette, mis à part l'agenda dans le ticket lié il n'y en a pas d'autre en cause.
Thomas Noël a écrit :
interdire au plus bas niveau possible, par exemple lors du save(), pour que ça crashe aussi lors d'un import
Pas besoin : le bug a bien été déclenché par un import, mais un import CSV, donc la validation normale suffit à s'en prémunir (cf test).
Mis à jour par Thomas Noël il y a presque 4 ans
Valentin Deniaud a écrit :
J'ai vérifié en prod et en recette, mis à part l'agenda dans le ticket lié il n'y en a pas d'autre en cause.
Thomas Noël a écrit :
interdire au plus bas niveau possible, par exemple lors du save(), pour que ça crashe aussi lors d'un import
Pas besoin : le bug a bien été déclenché par un import, mais un import CSV, donc la validation normale suffit à s'en prémunir (cf test).
Nope, pas de CSV, j'ai fait un import via export-site/import-site, donc en JSON. Ça râlera ?
Mis à jour par Valentin Deniaud il y a presque 4 ans
Thomas Noël a écrit :
Nope, pas de CSV, j'ai fait un import via export-site/import-site, donc en JSON. Ça râlera ?
Je parlais de la manière dont est survenu le bug dans le ticket client. Oui ça râlera, mais je ne vois pas comment on pourrait se retrouver avec de tels fichiers d'import une fois le problème résolu par ce patch ?
Mis à jour par Thomas Noël il y a presque 4 ans
Valentin Deniaud a écrit :
Thomas Noël a écrit :
Nope, pas de CSV, j'ai fait un import via export-site/import-site, donc en JSON. Ça râlera ?
Je parlais de la manière dont est survenu le bug dans le ticket client.
C'est vraiment comme ça que c'est arrivé sur agendas-mably.test.entrouvert.org, c'est moi qui a géré toute la migration des agenda, en ligne de commande.
Oui ça râlera, mais je ne vois pas comment on pourrait se retrouver avec de tels fichiers d'import une fois le problème résolu par ce patch ?
Parce qu'il y a de l'existant foireux qui ne va pas s'auto-corriger. Par exemple si j'exporte https://agendas-mably.test.entrouvert.org/manage/agendas/14/settings ...
Mis à jour par Valentin Deniaud il y a presque 4 ans
- Fichier 0001-agendas-forbid-numerical-slug-for-events-43789.patch 0001-agendas-forbid-numerical-slug-for-events-43789.patch ajouté
L'existant foireux, ce sont deux agendas sur l'ensemble du saas, je les ai signalés au client qui devrait les supprimer, je trouve dommage de faire du code pour si peu.
Mais j'ai mal répondu à la question précédente : non ça ne gueulait pas, j'ajoute donc un assert pour que ça gueule, et voilà le travail.
Mis à jour par Thomas Noël il y a presque 4 ans
- Statut changé de Solution proposée à Solution validée
Ca marche comme ça. Les petits malins qui voudront forger des json invalides se prendront des 500, mais on s'en fiche, et ça nous fera une alerte.
Mis à jour par Valentin Deniaud il y a presque 4 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 0d028dcdf20a018114f96389972c105f43e234b2
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu Jun 11 15:21:36 2020 +0200
agendas: forbid numerical slug for events (#43789)Mis à jour par Frédéric Péters il y a presque 4 ans
- Statut changé de Résolu (à déployer) à Solution déployée
agendas: forbid numerical slug for events (#43789)