megahertz / react-simple-wysiwyg

Simple and lightweight React WYSIWYG editor
https://megahertz.github.io/react-simple-wysiwyg/
MIT License
119 stars 21 forks source link

Использовать dangerouslySetInnerHTML - очень плохо. #1

Closed Fi1osof closed 4 years ago

Fi1osof commented 4 years ago

Причины две:

  1. Безопасность. Критическая проблема. Можно писать теги в том числе с событиями onclick и т.п.
  2. Производительность. При таком рендеринге реакт не отслеживает инстансы создаваемых HTML-нод, каждый раз создавая новые при ререндеринге.
megahertz commented 4 years ago

@Fi1osof Как вы представляете себе легкий WYSIWYG редактор без этого?

Fi1osof commented 4 years ago

@megahertz , сорри за долгий ответ.

Если предполагается использовать редактор только для того, чтобы отредактировать текст, но полученную разметку сохранять и выводить потом через сторонний механизм (например, средствами классической CMS), то да, скорее всего вариантов никаких других. Более того, даже рендеринг на уровне редактора не спасает от инъекций, если серверная часть принимает код полученный как есть, то есть позволяет прислать злоумышленнику что угодно. Но если на конечном сайте код выводится все-таки средствами редактора, можно все-таки что-то придумать. К примеру, у меня вот так делается. То есть есть реакт-компонент HtmlTag и через него рендерится каждая HTML-нода. Пока тоже еще версия не стабильная, но во всяком случае позволяет не только не париться на счет безопасности (ее реакт обеспечивает), но и при желании модифицировать вывод. Вот небольшое видео как работала первая версия.

А вот сегодня я уже боле менее полноценный редактор выкатил: https://prisma-cms.com/topics/contenteditor.html

Сорри за типа рекламу, но может и вам там там что-нибудь полезное найдется. К слову, советую тоже oninput заменить на MutationObserver. Это позволит более четко отслеживать все изменения. В моей статье про это тоже написано. У вас сейчас не фиксируются изменения, выполняемые программно, в том числе через devTools. Вот записал проблему.

megahertz commented 4 years ago

@Fi1osof Спасибо за информацию. Вкурсе про MutationObserver. Целенаправленно не стал использовать, чтоб не городить полифилы для старых IE. Мои потребности текущее решение полностью покрывает, а усложнять решение не вижу смысла пока нет особой востребованности библиотеки. В приоритете другие проекты.

Fi1osof commented 4 years ago

Не вопрос. Спасибо за ответы.