Debuggage des 404 massives

Lephenixnoir commented

2022-11-15 11:06:18 +01:00

Owner

Le bug est fixé, ceci est just un log pour les infos. @Darks @Eragon

Récemment le forum a des 404 presque partout (avec l'effet comique de valider en apparence la suppression des contenus par les bots d'index, un autre problème à trouver).

Les exemples observés sont :

https://v5.planet-casio.com/forum/discussion/6085 (conversion de discussion comme forum) : OK.
https://v5.planet-casio.com/forum/discussion/6085/1/test23 (conversion de discussion comme forum et 1/test23 comme page/slug) : 404.
https://v5.planet-casio.com/forum/discussion/ (conversion de discussion comme forum) : OK.
https://v5.planet-casio.com/forum/actus/projets (conversion de actus/projets comme forum) : 404.
https://v5.planet-casio.com/forum/discussion/6085/1 est invalide parce qu'on ne convertit que page/slug ensemble, pas de page tout seul, donc 404 attendu.

Symptôme : quand la chaîne à convertir contient un /, ça plante.

Reproduction minimale (dossier vide avec just app.py):

from flask import Flask
from werkzeug.routing import BaseConverter

app = Flask(__name__)

class TestConverter(BaseConverter):
	regex = '.*'

    def to_python(self, url):
        return url.split("/")

    def to_url(self, t):
        return "/".join(t)

app.url_map.converters['test'] = TestConverter

@app.route('/test/<test:t>')
def route_test(t):
    return str(t)

% curl 127.0.0.1:5000/test/a
['a']
% curl 127.0.0.1:5000/test/a/b
(404)

Indication pertinente dans le changelog Werkzeug: Fix router to restore the 2.1 strict_slashes == False behaviour whereby leaf-requests match branch rules and vice versa. #2489

Mais je suis plus intéressé par l'issue liée #2491 : The 2.2.x matcher no longer supports regexes that can match a slash.

Il apparaît que la version 2.2 introduit un nouveau parser d'URLs plus rapide, lequel n'accepte pas les slashs dans les chaînes à convertir par défaut.

La documentation indique qu'il existe maintenant un attribut appelé part_isolating qui est vrai par défaut et doit être faux pour que le convertisseur puisse voir les slashs.

Le fix est donc d'ajouter cet attribut, que l'on peut fixer constant au niveau de la classe :

class TestConverter(BaseConverter):
    regex = '.*'
    part_isolating = False
    # (...)

% curl 127.0.0.1:5000/test/a/b/c/d
['a', 'b', 'c', 'd']

Fixé par 2b9ab64f6e.

Le bug est fixé, ceci est just un log pour les infos. @Darks @Eragon Récemment le forum a des 404 presque partout (avec l'effet comique de valider en apparence la suppression des contenus par les bots d'index, un autre problème à trouver). Les exemples observés sont : * https://v5.planet-casio.com/forum/discussion/6085 (conversion de `discussion` comme forum) : OK. * https://v5.planet-casio.com/forum/discussion/6085/1/test23 (conversion de `discussion` comme forum et `1/test23` comme page/slug) : 404. * https://v5.planet-casio.com/forum/discussion/ (conversion de `discussion` comme forum) : OK. * https://v5.planet-casio.com/forum/actus/projets (conversion de `actus/projets` comme forum) : 404. * https://v5.planet-casio.com/forum/discussion/6085/1 est invalide parce qu'on ne convertit que page/slug ensemble, pas de page tout seul, donc 404 attendu. Symptôme : quand la chaîne à convertir contient un `/`, ça plante. ---- Reproduction minimale (dossier vide avec just `app.py`): ```python from flask import Flask from werkzeug.routing import BaseConverter app = Flask(__name__) class TestConverter(BaseConverter): regex = '.*' def to_python(self, url): return url.split("/") def to_url(self, t): return "/".join(t) app.url_map.converters['test'] = TestConverter @app.route('/test/<test:t>') def route_test(t): return str(t) ``` ``` % curl 127.0.0.1:5000/test/a ['a'] % curl 127.0.0.1:5000/test/a/b (404) ``` --- Indication pertinente dans le [changelog Werkzeug](https://werkzeug.palletsprojects.com/en/2.2.x/changes/): *Fix router to restore the 2.1 `strict_slashes == False` behaviour whereby leaf-requests match branch rules and vice versa. [#2489](https://github.com/pallets/werkzeug/pull/2489)* Mais je suis plus intéressé par l'issue liée [#2491](https://github.com/pallets/werkzeug/issues/2491) : *The 2.2.x matcher no longer supports regexes that can match a slash.* Il apparaît que la version 2.2 introduit un nouveau parser d'URLs plus rapide, lequel n'accepte pas les slashs dans les chaînes à convertir par défaut. La [documentation](https://werkzeug.palletsprojects.com/en/2.2.x/routing/#custom-converters) indique qu'il existe maintenant un attribut appelé `part_isolating` qui est vrai par défaut et doit être faux pour que le convertisseur puisse voir les slashs. --- Le fix est donc d'ajouter cet attribut, que l'on peut fixer constant au niveau de la classe : ```python class TestConverter(BaseConverter): regex = '.*' part_isolating = False # (...) ``` ``` % curl 127.0.0.1:5000/test/a/b/c/d ['a', 'b', 'c', 'd'] ``` Fixé par https://gitea.planet-casio.com/devs/PCv5/commit/2b9ab64f6ef88300032b671b246297688ff4a359.

Lephenixnoir closed this issue

2022-11-15 11:41:35 +01:00

Eragon commented

2022-11-15 14:29:46 +01:00

Member

Est-ce que part_isolating a un impact sur la performance du parser ? Et si il y en a un, est-ce qu'il est possible de convertir notre regex pour ne pas avoir cet impact ?

Lephenixnoir commented

2022-11-15 14:38:48 +01:00

Author

Owner

L'impact c'est qu'on n'a pas le speedup de la 2.2. Pour le cas du convertisseur page comme on a un unique / on pourrait le séparer en deux, cela dit le but d'en avoir un seul c'était de pas bloat les paramètres des routes. C'est discutable.

Pour les forums c'est pas possible sauf si on énumère les deux case possibles (pour la route des forums, et pour la route des topics, ce qui se défend).

Sinon on peut juste mettre autre chose que des slashs comme séparateurs.

Mais je vois pas dans quel monde c'est ça notre bottleneck, franchement. La dernière fois j'ai viré 50% du temps de réponse d'une page du forum entièrement sur le niveau ORM en ajustant les paramètres de génération des requêtes. C'est donc certainement là et sur le rendu Jinja qu'il reste le plus de temps passé...

L'impact c'est qu'on n'a pas le speedup de la 2.2. Pour le cas du convertisseur `page` comme on a un unique / on pourrait le séparer en deux, cela dit le but d'en avoir un seul c'était de pas bloat les paramètres des routes. C'est discutable. Pour les forums c'est pas possible sauf si on énumère les deux case possibles (pour la route des forums, et pour la route des topics, ce qui se défend). Sinon on peut juste mettre autre chose que des slashs comme séparateurs. Mais je vois pas dans quel monde c'est ça notre bottleneck, franchement. La dernière fois j'ai viré 50% du temps de réponse d'une page du forum entièrement sur le niveau ORM en ajustant les paramètres de génération des requêtes. C'est donc certainement là et sur le rendu Jinja qu'il reste le plus de temps passé...

Debuggage des 404 massives #116