Skip to content

Performance : corriger le DoS sur l'emphase (*, _) et limiter la profondeur d'imbrication - #41

Merged
jmcollin merged 1 commit into
mainfrom
fix/emphasis-parsing-dos
Oct 8, 2026
Merged

jmcollin merged 1 commit into
mainfrom
fix/emphasis-parsing-dos

Conversation

@jmcollin

@jmcollin jmcollin commented Oct 7, 2026

Copy link
Copy Markdown
Owner

Problème

Une entrée d'emphase pathologique rendait le parsing inline quadratique et pouvait faire planter le process :

Entrée main Cette PR
"*a " × 50 000 (150 Ko) ~36 s < 0,3 s
"_a" × 50 000 ~33 s < 0,2 s
*a × 30 000 puis a* × 30 000 (paires imbriquées) Fatal : Maximum call stack size ~0,3 s

À la limite par défaut de 1 Mio, on dépassait largement la minute.

Correctif : trois points chauds quadratiques

  1. DelimiterStack est réécrit selon l'algorithme de l'annexe A de CommonMark (process emphasis) : listes doublement chaînées, table openers_bottom, et emballage d'une plage en <em>/<strong> par réassignation de liens en O(1). L'ancienne version utilisait array_slice/array_splice, un count() à chaque tour et un resolve() récursif. La règle du multiple de 3 utilise maintenant les longueurs d'origine des runs, comme l'exige la spec. La fusion finale du texte passe par un tampon pour éviter les re-copies de chaînes.
  2. FlankingComputer lit les caractères UTF-8 voisins directement aux offsets en octets. Avant, il faisait mb_strlen(substr($text, 0, $pos)) pour chaque délimiteur, soit O(n) par run.
  3. InlineParser::flushBuffer() prend $tokens par référence. Le passage par valeur suivi d'un retour copiait tout le tableau à chaque flush.

L'imbrication emphase/strong est plafonnée à 64 niveaux. Au-delà, les délimiteurs sont rendus en texte littéral, ce qui garde l'AST et le renderer récursif peu profonds.

Conformité

Sur un corpus aléatoire de 20 000 chaînes d'emphase, comparé à markdown-it (référence CommonMark) :

  • main : 18 970 / 20 000 identiques
  • cette PR : 19 384 / 20 000

Les seules sorties modifiées qui diffèrent encore le font à cause de problèmes de code span déjà présents avant cette PR.

Tests

  • Nouveau tests/Security/EmphasisDosTest.php : trois tests de temps et de profondeur, plus un test d'imbrication normale. Ils échouent sur main (36 s, 33 s, crash) et passent ici en ~0,7 s avec moins de 128 Mo de mémoire.
  • PHPUnit : 837 tests OK. PHPStan et Psalm : aucune erreur.

Hors périmètre, à traiter à part : les références raccourcies [text] restent quadratiques quand le document contient une définition de lien et beaucoup de [ sans ] (200 Ko → ~2,5 s).

🤖 Generated with Claude Code

https://claude.ai/code/session_01CPcuaaxrCottkeL1ykoSWe


Generated by Claude Code

Pathological emphasis input made inline parsing quadratic and could crash
the process: 60 KB of "*a " took ~4.5 s (minutes at the 1 MiB default
limit) and ~20k nested pairs overflowed the stack in the renderer.

Three quadratic hot spots are removed:
- DelimiterStack: rewritten as the CommonMark Appendix A algorithm over
  linked lists with an openers_bottom table; wrapping a span relinks it in
  O(1) instead of array_slice/array_splice + recursive resolve(). The
  multiple-of-3 rule now uses original run lengths, as the spec requires.
- FlankingComputer: read neighbouring UTF-8 chars from byte offsets
  instead of mb_strlen() of the whole prefix for every delimiter run.
- InlineParser::flushBuffer(): take $tokens by reference instead of
  copying the whole array on every flush.

Emphasis nesting is capped at 64 levels; deeper pairs are emitted as
literal delimiters, keeping the AST and the recursive renderer shallow.

On a 20k-case random emphasis corpus, agreement with the CommonMark
reference (markdown-it) goes from 18970 to 19384 cases; the only changed
outputs that still differ do so because of pre-existing code-span issues.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CPcuaaxrCottkeL1ykoSWe
@jmcollin
jmcollin merged commit 3ac7161 into main Oct 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants