RateLimiterLive : cast des IPs en string #3659
Conversation
|
|
||
| test "ips_in_jail" do | ||
| assert [] == TransportWeb.Backoffice.RateLimiterLive.ips_in_jail() | ||
| PhoenixDDoS.Jail.send(~c"108.128.238.17", {nil, %{jail_time: {1, :hour}}}) |
There was a problem hiding this comment.
Semble être le format utilisé par la lib, j'avais dans mes tests mis ça en string.
https://github.com/xward/phoenix_ddos/blob/feb07469ce318214cddb8e88ac18b5f94b3e31f2/lib/phoenix_ddos/template/engine.eex#L42
:inet.ntoa() donne un charlist
{127, 0, 0, 1} |> :inet.ntoa()
~c"127.0.0.1"Merge queue setting changed
There was a problem hiding this comment.
Bien vu !
Dommage, la librairie est mal documentée
Je le vois autrement : si je comprends bien, notre code fait appel à Cachex en direct pour avoir les clés.
Or, ce point n'est pas mentionné dans la doc publique (https://github.com/xward/phoenix_ddos).
Donc on est en train d'appeler une API privée de la librairie, qui ne fournit aucune garantie (et qui pourrait changer librement en tant que mainteneur à chaque release). On se protège de ça avec les tests (et ça c'est cool).
Suggestions pour améliorer ça:
- mettre un commentaire clair dans le code sur cet appel à Cachex pour indiquer que c'est "instable / API privée / non documenté"
- ouvrir un ticket sur la librairie pour remonter le besoin de ce type (il pourrait y avoir un wrapper qui convertit et expose les clés, directement là dedans ; il y a partiellement ça déjà dans les helpers de test de la librairie
There was a problem hiding this comment.
Top, merci @AntoineAugusti pour le soin apporté là dessus.
Mea culpa sur mon commentaire qui aurait pu être plus lissé on va dire, j'ai un peu mal à la tête, ceci explique peut-être un peu cela.
|
|
Suite de #3652.
Répare une exception Sentry, les clés Cachex étaient stockées en tant que character list
~c. Dommage, la librairie ne documente pas ceci. J'en profite pour ajouter pas mal de tests 🧑🏭 ✅