Maximale Verschachtelungstiefe für aufklappbare Details hinzufügen

[scavin]

Ich möchte vorschlagen, eine maximale Verschachtelungstiefe für die eingebaute [details]-Funktion hinzuzufügen oder eine Seiteneinstellung einzuführen, mit der Administratoren dieses Limit konfigurieren können.

Das Problem

Kürzlich hat ein Benutzer auf meinem Discourse-Forum einen Beitrag erstellt, der 24 Ebenen verschachtelter [details]-Abschnitte enthielt.

Dies führte zu mehreren Problemen:

1. Browser-Performance: Benutzer meldeten, dass das Aufklappen mehrerer Ebenen dazu führte, dass Firefox extrem langsam oder nicht mehr ansprechend wurde.
2. Timeouts der Markdown-Endpunkte: Der Zugriff auf den .md-Endpunkt des Themas wurde extrem langsam. Wir beobachteten Anfragen, die 19,8 Sekunden und 31,1 Sekunden in Anspruch nahmen.
3. Serverfehler: Der Markdown-Endpunkt gab manchmal eine „Oops“-Fehlerseite zurück. Die Discourse-Logs zeigten während der Markdown-Konvertierung auch Pitchfork-Worker-Timeout-Warnungen.
4. Potenzielle Ressourcenerschöpfung: Da Suchmaschinen und KI-Crawler häufig .md-Endpunkte anfordern, könnte ein stark verschachtelter Beitrag wiederholt aufwendige Verarbeitungsschritte auslösen und Serverressourcen verbrauchen.

Nach dem Entfernen des problematischen Beitrags gab der Markdown-Endpunkt HTTP 200 zurück und reagierte in etwa 1,7 Sekunden.

Das betroffene Thema war:

https://meta.appinn.net/t/topic/87672

(Die problematische Antwort wurde bereits entfernt.)

Vorschlag zur Verbesserung

Ich denke, es wäre nützlich, entweder:

• Die Verschachtelung von [details] auf eine angemessene Tiefe zu begrenzen, z. B. 2 oder 3 Ebenen.
• Eine Seiteneinstellung hinzuzufügen, wie z. B. details_max_nesting_depth, die es Administratoren ermöglicht, die maximale Verschachtelungstiefe zu konfigurieren.

Zur Abwärtskompatibilität könnte die Einstellung standardmäßig auf unbegrenzt gesetzt sein, während Administratoren bei Bedarf ein Limit durchsetzen können.

Wenn Benutzer das konfigurierte Limit überschreiten, könnte Discourse den Beitrag mit einer klaren Validierungsnachricht ablehnen.

Temporäre Lösung: ein Plugin

Ich habe ein kleines Plugin erstellt, das die Verschachtelung von [details] auf maximal 2 Ebenen begrenzt:

GitHub - scavin/discourse-details-depth-limit · GitHub

Das Plugin validiert Beiträge serverseitig und lehnt Beiträge ab, die mehr als zwei Ebenen verschachtelter aufklappbarer Abschnitte enthalten.

Ich habe es erfolgreich in meiner lokalen Discourse-Entwicklungsumgebung getestet.

Für Administratoren, die ähnliche Probleme erleben, kann dieses Plugin als vorübergehende Lösung dienen, bis eine offizielle Lösung verfügbar ist.

Ich bin der Meinung, dass ein konfigurierbares Limit im eingebauten Details-Plugin eine nützliche Schutzmaßnahme gegen versehentliche oder übermäßige Verschachtelung wäre.

[Ethsim2 (Ethan)]

Hallo @scavin, danke für deinen Hinweis und die Bereitstellung der Replikationsdetails.

Ich habe die von dir beschriebene Verlangsamung des Markdown-Endpunkts untersucht und ein Problem in MarkdownEndpoint::CookedProcessor#replace_details identifiziert: Verschachtelte <details>-Elemente wurden wiederholt konvertiert, was zu einer exponentiellen Verarbeitungsdauer führte.

Ich habe einen Entwurf für einen Upstream-PR mit einer Lösung erstellt:

https://github.com/discourse/discourse/pull/44461

Bei acht Verschachtelungsebenen reduzierte die Lösung die rekursiven Konvertierungen von 255 auf 8, was in meinem lokalen Benchmark zu einer Geschwindigkeitssteigerung von etwa 31× führte. Der PR hat die ersten GitHub-CI-Prüfungen bestanden.

Ich arbeite außerdem an deinem vorgeschlagenen Site-Einstellung details_max_nesting_depth innerhalb der eingebauten Details-Plugin, wobei 0 für unbegrenzte Verschachtelung verwendet wird, um das bestehende Verhalten beizubehalten. Die erste Implementierung ist geschrieben, aber ihre Integrationstests sind noch im Gange.

Die Leistungskorrektur behebt das Problem der serverseitigen Markdown-Konvertierung; die konfigurierbare Grenze würde einen zusätzlichen Schutz bieten, insbesondere für die Leistung auf der Browserseite.

Ich würde Feedback zur vorgeschlagenen Einstellung begrüßen und ob es vorzuziehen wäre, sie in denselben PR aufzunehmen oder sie separat einzureichen.

[scavin]

Danke für die schnelle Untersuchung und Behebung!
Ich denke, es wäre besser, sie in zwei PRs zu trennen, damit der Performance-Fix unabhängig und ohne Warten auf die neue Einstellung geprüft und gemergt werden kann.
Ich unterstütze auch die konfigurierbare Tiefenbegrenzung, wobei 0 als Standardwert für die Abwärtskompatibilität vorgesehen ist.

[Ethsim2 (Ethan)]

Danke @scavin. Ich sehe den Vorteil darin, sie zu trennen, auch wenn sich beide Änderungen auf denselben ursprünglichen Bericht beziehen und mittlerweile implementiert und per CI getestet sind. Ich habe bei all meinen PRs Bearbeitungen durch Maintainer aktiviert, daher bin ich offen für die Präferenz des Discourse-Teams – egal ob beide Commits gemeinsam überprüft oder getrennt werden sollen.

[zogstrip]

Danke an @scavin für den Report und an @Ethsim2 für den PR Ich habe den PR zudem schlanker gemacht und die maximale Verschachtelungstiefe auf 10 gesetzt. Ich bin mir nicht sicher, ob wir je mehr als das unterstützen müssen.