Sanitizer features #467

khaydarov · 2018-10-07T23:03:20Z

No description provided.

gohabereg · 2018-10-16T21:02:06Z

src/components/modules/saver.ts

+ * When Tool's "inlineToolbar" value is True, get all sanitizer rules from all tools,
+ * otherwise get only enabled
+ */
+ private getSanitizerConfig(name) {


Лучше вынести в санитайзер

gohabereg · 2018-10-16T21:02:39Z

src/components/modules/sanitizer.ts

+ * Sets sanitizer configuration. Uses default config if user didn't pass the restriction
+ */
+ public get defaultConfig() {
+ return {


От дефолтом теперь же можно избавиться

а как же санитайз ядра при вставке? в плагинах sanitize - опциональное поле. Оно применяется при сохранении, но не при вставке

Он будет сформирован инлайн тулами

а, да, ок

neSpecc · 2018-10-17T11:50:32Z

src/components/modules/saver.ts

+ */
+ private getSanitizerConfig(name) {
+ const toolsConfig = this.Editor.Tools.getToolSettings(name),
+ enableInlineTools = toolsConfig.inlineToolbar || [];


к настройкам плагинов надо обращаться через Tools.apiSettings.IS_ENABLED_INLINE_TOOLBAR

enabledInlineTools

neSpecc · 2018-10-17T11:55:07Z

docs/api.md

@@ -34,11 +34,16 @@ Methods that working with Blocks

 ```insertNewBlock()``` - insert new Block after working place

+```getSanitizerConfig()``` - method return allowed inline-tool's sanitizer rules


этот метод работает только с инлайн тулами. То есть это нельзя назвать sanitizerConfig, тк там нету конфета самого плагина. Он должен либо отдавать полный конфиг (sanitize плагина + sanitize инлайн тулов), либо называться getInlineToolsSanitizerConfig. Первый вариант лучше

neSpecc · 2018-10-17T11:55:55Z

src/components/modules/saver.ts

+ * getting only enabled
+ */
+ enableInlineTools.map( (inlineToolName) => {
+ config = Object.assign(config, this.Editor.InlineToolbar.tools.get(inlineToolName).sanitize);


надо учесть, что sanitize — это опциональное поле, то есть его может не быть. И в инлайн тулах тоже.

neSpecc · 2018-10-17T11:56:47Z

src/components/modules/saver.ts

+ /**
+ * getting all tools sanitizer rule
+ */
+ this.Editor.InlineToolbar.tools.forEach( (inlineTool) => {


почему тут inlineTool, а ниже inlineToolName ? лучше везде inlineToolName

neSpecc · 2018-10-17T11:59:33Z

src/components/modules/saver.ts

+ cleanData;
+
+ blocks.forEach((block) => {
+ baseConfig = this.getSanitizerConfig(block.name);


лучше, чтобы метод getSanitizerConfig возвращал уже смердженный конфиг санитайза (а не только инлайн-тулы). То есть, вся логика по формированию конфига будет в одном месте. И код deepSanitize упростится за счет того, что не будет baseConfig

neSpecc · 2018-10-17T12:03:02Z

src/components/modules/sanitizer.ts

+ *
+ * @return {String} clean HTML
+ */
+ public clean(taintString: string, customConfig: ISanitizerConfig) {


надо указать возвращаемые значения функций

neSpecc · 2018-10-17T12:03:44Z

src/components/modules/sanitizer.ts

+ * @param {Object} rules - object with sanitizer rules
+ * @param {Object} baseConfig - object with sanitizer rules from inline-tools
+ */
+ public deepSanitize(blockData, rules, baseConfig) {


нужно указать тип параметров и возвращаемого значения

neSpecc · 2018-10-17T12:04:02Z

src/components/modules/sanitizer.ts

+ /**
+ * Method recursively reduces Block's data and cleans with passed rules
+ *
+ * @param {Object|string} blockData - taint string or object/array that contains taint string


у нас есть интерфейс IBlockData или вроде того

neSpecc · 2018-10-17T12:04:17Z

src/components/modules/sanitizer.ts

+ * Method recursively reduces Block's data and cleans with passed rules
+ *
+ * @param {Object|string} blockData - taint string or object/array that contains taint string
+ * @param {Object} rules - object with sanitizer rules


такой интерфейс тоже есть вроде

neSpecc · 2018-10-17T12:04:56Z

src/components/modules/sanitizer.ts

+ for (const data in blockData) {
+ if (Array.isArray(blockData[data]) || typeof blockData[data] === 'object') {
+ /**
+ * Case 1 & Case 2


пишешь, что два кейса, но в коде три кейса: if, elseif, else. Не понятно что тогда else.

package.json

neSpecc · 2018-10-19T14:00:27Z

src/components/modules/sanitizer.ts

+ super({config});
+
+ /** Custom configuration */
+ this.sanitizerConfig = config.settings ? config.settings.sanitizer : null;


актуализировать

src/components/modules/sanitizer.ts

neSpecc · 2018-10-19T14:02:07Z

src/components/modules/sanitizer.ts

+ return blocksData.map((block) => {
+ toolClass = this.Editor.Tools.toolsAvailable[block.tool];
+
+ const output = {


можно убрать

neSpecc · 2018-10-19T14:02:53Z

src/components/modules/sanitizer.ts

+ /**
+ * Enable sanitizing if Tool provides config
+ */
+ if (toolClass.sanitize && !_.isEmpty(toolClass.sanitize)) {


развернуть иф

neSpecc · 2018-10-19T14:08:59Z

src/components/modules/sanitizer.ts

+ * 2) If rule is false, which means clean all
+ * 3) Do nothing
+ */
+ if (rules[data]) {


вынести иф

neSpecc · 2018-10-19T14:09:05Z

src/components/modules/sanitizer.ts

+ * 3. When Data is base type (string, int, bool, ...). Check if rule is passed
+ */
+ for (const data in blockData) {
+ if (Array.isArray(blockData[data])) {


вынести иф

neSpecc · 2018-10-19T14:10:16Z

src/components/modules/sanitizer.ts

+ * Merge with inline tool config
+ *
+ * @param {string} toolName
+ * @param {ISanitizerConfig} blockRules


neSpecc · 2018-10-19T14:12:18Z

src/components/modules/sanitizer.ts

+
+ const toolConfig = {};
+ for (const blockRule in blockRules) {
+ if (blockRules[blockRule]) {


blockRules[blockRule] !== false

учесть true

gohabereg

Потести еще paste, чтобы все ок было с санитайзингом

package.json

neSpecc · 2018-10-19T18:14:26Z

src/components/modules/paste.ts

@@ -152,7 +152,7 @@ export default class Paste extends Module {
 return result;
 }, {});

- const customConfig = Object.assign({}, toolsTags, Sanitizer.defaultConfig.tags);
+ const customConfig = Object.assign({}, toolsTags, Sanitizer.defaultConfig);


обсудить с @gohabereg

neSpecc · 2018-10-19T18:14:50Z

src/components/modules/sanitizer.ts

+ */
+ public deepSanitize(blockData, rules, depth = 0) {
+
+ console.log('start: depth', depth);


neSpecc · 2018-10-19T18:15:16Z

src/components/modules/sanitizer.ts

+ * @param {ISanitizerConfig} rules - object with sanitizer rules
+ * @param {number} depth
+ */
+ public deepSanitize(blockData, rules, depth = 0) {


remove deph

neSpecc · 2018-10-19T18:15:49Z

src/components/modules/sanitizer.ts

+ *
+ * 1) Array - we need to save as array, thats why we do Array.map and call itself recursively
+ * 2) Object - we make new object with clean data and call itself
+ * 3) Basic type - we clean data


Boolean/string/number

neSpecc · 2018-10-19T18:19:54Z

src/components/modules/sanitizer.ts

+ * 2) Object - we make new object with clean data and call itself
+ * 3) Basic type - we clean data
+ */
+ for (const data in blockData) {


data -> fieldName

neSpecc · 2018-10-19T18:22:41Z

src/components/modules/sanitizer.ts

+ * - if passed config is not valid, call itself with parent's config
+ */
+ if (this.isConfigValid(rules[data])) {
+ cleanData[data] = currentIterationItem.map( (arrayData) => {


arrayData -> arrayItem

Sanitizer features

4fec735

khaydarov requested review from gohabereg, neSpecc and talyguryn as code owners October 7, 2018 23:03

khaydarov added 2 commits October 16, 2018 19:36

Merge branch 'master' into sanitizer-new-features

afd38cd

move to ts, use sanitizer module to clean taintstring

12f7ba2

gohabereg requested changes Oct 16, 2018

View reviewed changes

neSpecc requested changes Oct 17, 2018

View reviewed changes

Sanitizer is ready

cf0d0b4

neSpecc requested changes Oct 19, 2018

View reviewed changes

gohabereg reviewed Oct 19, 2018

View reviewed changes

package.json Show resolved Hide resolved

khaydarov and others added 2 commits October 19, 2018 21:07

it works

a1f8dd7

Code refactored (#476)

b010152

neSpecc requested changes Oct 20, 2018

View reviewed changes

gohabereg and others added 11 commits October 20, 2018 17:14

Use new features for paste handling

3c0362c

Fix asterix

eefc42e

Add types and some comments

4bc1bfe

Use any type for deepSanitize method

07487b8

Make sanitize property static and use apiSettings object

631016a

Use sanitize for single-block pasting

af56386

Fix comment

a093351

little updates

650879e

rename sanitize in inline-tools docs

b6b762b

Update pattern handling

b6ec678

Use public getter for available tools

c1d7d14

neSpecc approved these changes Oct 23, 2018

View reviewed changes

Fix typo

65dd037

gohabereg approved these changes Oct 23, 2018

View reviewed changes

khaydarov merged commit 3f8c7fb into master Oct 23, 2018

khaydarov deleted the sanitizer-new-features branch October 23, 2018 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sanitizer features #467

Sanitizer features #467

khaydarov commented Oct 7, 2018

gohabereg Oct 16, 2018

gohabereg Oct 16, 2018

neSpecc Oct 16, 2018

gohabereg Oct 16, 2018 •

edited

Loading

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 17, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018 •

edited

Loading

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

gohabereg left a comment

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

neSpecc Oct 19, 2018

		@@ -34,11 +34,16 @@ Methods that working with Blocks

		```insertNewBlock()``` - insert new Block after working place

		```getSanitizerConfig()``` - method return allowed inline-tool's sanitizer rules

Sanitizer features #467

Sanitizer features #467

Conversation

khaydarov commented Oct 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gohabereg Oct 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neSpecc Oct 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gohabereg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gohabereg Oct 16, 2018 •

edited

Loading

neSpecc Oct 19, 2018 •

edited

Loading