Добавить максимальную глубину вложенности для сворачиваемых деталей

Хотел бы предложить установить максимальную глубину вложенности для встроенной функции [details] или ввести настройку сайта, позволяющую администраторам настраивать этот лимит.

Проблема

Недавно пользователь на моём форуме Discourse создал сообщение, содержащее 24 уровня вложенных секций [details].

Это привело к нескольким проблемам:

  1. Производительность браузера: Пользователи сообщали, что раскрытие нескольких уровней приводило к тому, что Firefox становился крайне медленным или переставал реагировать.
  2. Тайм-ауты Markdown-эндпоинта: Доступ к эндпоинту .md темы стал крайне медленным. Мы наблюдали запросы, занимавшие 19,8 и 31,1 секунды.
  3. Ошибки сервера: Эндпоинт Markdown иногда возвращал страницу ошибки «Oops». В логах Discourse также появлялись предупреждения о тайм-аутах воркеров Pitchfork во время конвертации Markdown.
  4. Потенциальное истощение ресурсов: Поскольку поисковые системы и ИИ-краулеры часто запрашивают эндпоинты .md, сообщение с глубокой вложенностью могло многократно запускать ресурсоёмкую обработку и потреблять ресурсы сервера.

После удаления проблемного сообщения эндпоинт Markdown возвращал HTTP 200 и отвечал примерно за 1,7 секунды.

Затронутая тема:

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

(Проблемный ответ уже удалён.)

Предлагаемое улучшение

Я считаю, что было бы полезно либо:

  • Ограничить вложенность [details] разумной глубиной, например 2 или 3 уровнями.
  • Добавить настройку сайта, такую как details_max_nesting_depth, позволяющую администраторам настраивать максимальную глубину вложенности.

Для обратной совместимости значение по умолчанию для этой настройки может быть установлено как «без ограничений», при этом администраторы смогут устанавливать лимит при необходимости.

Если пользователи превышают настроенный лимит, Discourse может отклонять сообщение с ясным сообщением об ошибке валидации.

Временное решение: плагин

Я создал небольшой плагин, который ограничивает вложенность [details] максимум 2 уровнями:

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

Плагин выполняет валидацию сообщений на стороне сервера и отклоняет сообщения, содержащие более двух уровней вложенных сворачиваемых секций.

Я успешно протестировал его в своей локальной среде разработки Discourse.

Для администраторов, сталкивающихся с аналогичными проблемами, этот плагин может служить временным решением до появления официального решения.

Я считаю, что настраиваемый лимит во встроенном плагине Details будет полезной мерой предосторожности против случайной или чрезмерной вложенности.

2 лайка

Привет, @scavin, спасибо за сообщение об этой проблеме и за предоставление деталей для воспроизведения.

Я расследовал замедление работы Markdown-эндпоинта, о котором вы сообщили, и обнаружил проблему в MarkdownEndpoint::CookedProcessor#replace_details: вложенные элементы <details> конвертировались многократно, что приводило к экспоненциальному росту времени обработки.

Я создал черновик PR upstream с исправлением:

При глубине вложенности в восемь уровней исправление сократило количество рекурсивных конвертаций с 255 до 8, что дало примерно 31-кратное ускорение в моих локальных бенчмарках. PR успешно прошел первоначальные проверки CI на GitHub.

Я также работаю над предложенным вами параметром настройки сайта details_max_nesting_depth в встроенном плагине Details, используя значение 0 для неограниченной вложенности, чтобы сохранить существующее поведение. Начальная реализация написана, но интеграционные тесты все еще в процессе.

Исправление производительности решает проблему серверной конвертации Markdown; настраиваемый лимит предоставит дополнительную защиту, особенно для производительности на стороне браузера.

Буду рад получить обратную связь по предложенному параметру и узнать, предпочтительнее ли включить его в тот же PR или отправить отдельно.

1 лайк

Спасибо за быстрое расследование и исправление!
Я думаю, что лучше разделить их на два PR, чтобы исправление производительности можно было просмотреть и объединить независимо, не дожидаясь новой настройки.
Я также поддерживаю настраиваемый предел глубины, со значением по умолчанию 0 для обратной совместимости.

Спасибо, @scavin. Я вижу преимущества в их разделении, хотя оба изменения касаются одного и того же исходного отчёта, и сейчас они уже реализованы и протестированы в CI. У меня включены правки мейнтейнера во всех моих PR, поэтому я готов следовать предпочтениям команды Discourse — будь то совместный обзор обоих коммитов или их разделение.

1 лайк