diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 480a12c7f..931051a1d 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -5,6 +5,7 @@ - `Improvement` - Removed "Narrow" ("Thin") mode. On non-mobile viewports, Plus and Block Tunes controls now use a responsive action rail beside the editor content. - `Improvement` - `paste` config supports filter functions for tag-based substitution - `Fix` - "Convert to" is not shown in the Inline Toolbar when there are no tools to convert to +- `Fix` - Sanitize rules for the same tag declared by several Inline Tools are merged instead of overwriting each other ### 2.31.7 diff --git a/package.json b/package.json index b57e61968..9e8894910 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@editorjs/editorjs", - "version": "2.32.0-rc.3", + "version": "2.32.0-rc.4", "description": "Editor.js — open source block-style WYSIWYG editor with JSON output", "main": "dist/editorjs.umd.js", "module": "dist/editorjs.mjs", diff --git a/src/components/modules/tools.ts b/src/components/modules/tools.ts index 618242697..548b8431c 100644 --- a/src/components/modules/tools.ts +++ b/src/components/modules/tools.ts @@ -1,6 +1,7 @@ import Paragraph from '@editorjs/paragraph'; import Module from '../__module'; import * as _ from '../utils'; +import { mergeSanitizerConfigs } from '../utils/sanitizer'; import type { SanitizerConfig, ToolConfig, ToolConstructable, ToolSettings } from '../../../types'; import BoldInlineTool from '../inline-tools/inline-tool-bold'; import ItalicInlineTool from '../inline-tools/inline-tool-italic'; @@ -150,14 +151,9 @@ export default class Tools extends Module { */ @_.cacheable public getAllInlineToolsSanitizeConfig(): SanitizerConfig { - const config: SanitizerConfig = {} as SanitizerConfig; - - Array.from(this.inlineTools.values()) - .forEach(inlineTool => { - Object.assign(config, inlineTool.sanitizeConfig); - }); - - return config; + return mergeSanitizerConfigs( + ...Array.from(this.inlineTools.values()).map(inlineTool => inlineTool.sanitizeConfig) + ); } /** diff --git a/src/components/tools/block.ts b/src/components/tools/block.ts index d56cc4321..3fcb57358 100644 --- a/src/components/tools/block.ts +++ b/src/components/tools/block.ts @@ -9,6 +9,7 @@ import type { ToolboxConfigEntry } from '@/types'; import * as _ from '../utils'; +import { mergeSanitizerConfigs } from '../utils/sanitizer'; import type InlineToolAdapter from './inline'; import type BlockTuneAdapter from './tune'; import ToolsCollection from './collection'; @@ -202,16 +203,9 @@ export default class BlockToolAdapter extends BaseToolAdapter Object.assign(baseConfig, tool.sanitizeConfig)); - - Array - .from(this.tunes.values()) - .forEach(tune => Object.assign(baseConfig, tune.sanitizeConfig)); - - return baseConfig; + return mergeSanitizerConfigs( + ...Array.from(this.inlineTools.values()).map(tool => tool.sanitizeConfig), + ...Array.from(this.tunes.values()).map(tune => tune.sanitizeConfig) + ); } } diff --git a/src/components/utils/sanitizer.ts b/src/components/utils/sanitizer.ts index 69d501490..37a29038e 100644 --- a/src/components/utils/sanitizer.ts +++ b/src/components/utils/sanitizer.ts @@ -30,9 +30,140 @@ import * as _ from '../utils'; */ import HTMLJanitor from 'html-janitor'; -import type { BlockToolData, SanitizerConfig } from '../../../types'; +import type { BlockToolData, SanitizerConfig, SanitizerRule } from '../../../types'; +import type { TagConfig } from '../../../types/configs/sanitizer-config'; import type { SavedData } from '../../../types/data-formats'; +/** + * Value of a single attribute inside a tag rule object + * + * html-janitor accepts: + * - true — keep the attribute with any value + * - false — strip the attribute + * - string — keep the attribute only if its value equals the string + * - function — keep the attribute only if the function returns true + */ +type AttributeRule = boolean | string | ((value: string, node: Element) => boolean); + +/** + * Merges several sanitizer configs into a single one, combining rules per tag + * instead of overwriting them + * + * Needed when several Inline Tools (or Block Tunes) declare rules for the same tag, + * e.g. two tools that both wrap text with but allow different attributes. + * With a plain Object.assign the last tool's rule would replace the first one. + * + * Merge semantics (a rule that allows something always wins over one that does not): + * - tag: true + anything → true (all attributes allowed) + * - tag: false (or missing) + X → X + * - tag: object + object → object with the union of attributes + * - tag: function + object/function → function returning the merged result of both for the element + * - attribute: true + anything → true + * - attribute: false (or missing) + X → X + * - attribute: equal strings → that string + * - attribute: any other mix of string/function → function that passes when either side passes + * + * Passed configs are not mutated. + * + * @param configs - sanitizer configs to merge + * @returns merged sanitizer config + */ +export function mergeSanitizerConfigs(...configs: Array): SanitizerConfig { + const result = {} as SanitizerConfig; + + configs.forEach((config) => { + if (!_.isObject(config)) { + return; + } + + Object.keys(config).forEach((tag) => { + result[tag] = mergeTagRules(result[tag], config[tag]); + }); + }); + + return result; +} + +/** + * Merges two rules for the same tag + * + * @param a - first rule + * @param b - second rule + */ +function mergeTagRules(a: SanitizerRule | undefined, b: SanitizerRule | undefined): SanitizerRule { + if (a === undefined || a === null || a === false) { + return b; + } + + if (b === undefined || b === null || b === false) { + return a; + } + + if (a === true || b === true) { + return true; + } + + if (_.isFunction(a) || _.isFunction(b)) { + return (el: Element): TagConfig => { + const resultA = _.isFunction(a) ? a(el) : a; + const resultB = _.isFunction(b) ? b(el) : b; + + return mergeTagRules(resultA, resultB) as TagConfig; + }; + } + + const merged = { ...a }; + + Object.keys(b).forEach((attr) => { + merged[attr] = mergeAttributeRules(merged[attr] as AttributeRule, b[attr] as AttributeRule) as boolean | string; + }); + + return merged; +} + +/** + * Merges two rules for the same attribute of a tag + * + * @param a - first rule + * @param b - second rule + */ +function mergeAttributeRules(a: AttributeRule | undefined, b: AttributeRule | undefined): AttributeRule { + if (a === undefined || a === null || a === false) { + return b; + } + + if (b === undefined || b === null || b === false) { + return a; + } + + if (a === true || b === true) { + return true; + } + + if (a === b) { + return a; + } + + return (value: string, node: Element): boolean => { + return checkAttributeRule(a, value, node) || checkAttributeRule(b, value, node); + }; +} + +/** + * Checks whether an attribute value passes a string or function rule + * + * @param rule - string (exact value) or function rule + * @param value - attribute value + * @param node - element that owns the attribute + */ +function checkAttributeRule(rule: AttributeRule, value: string, node: Element): boolean { + if (_.isFunction(rule)) { + return rule(value, node) === true; + } + + return rule === value; +} + /** * Sanitize Blocks * diff --git a/test/cypress/tests/sanitisation.cy.ts b/test/cypress/tests/sanitisation.cy.ts index f0653b870..7b06ea8be 100644 --- a/test/cypress/tests/sanitisation.cy.ts +++ b/test/cypress/tests/sanitisation.cy.ts @@ -1,8 +1,8 @@ import type EditorJS from '../../../types/index'; -import type { OutputData } from '../../../types/index'; +import type { OutputData, SanitizerConfig } from '../../../types/index'; -/* eslint-disable @typescript-eslint/no-explicit-any */ +/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/naming-convention */ describe('Sanitizing', () => { context('Output should save inline formatting', () => { it('should save initial formatting for paragraph', () => { @@ -75,6 +75,84 @@ describe('Sanitizing', () => { }); }); + /** + * @see https://github.com/codex-team/editor.js/issues/2501 + */ + it('should keep attributes allowed by every inline tool when several tools use the same tag', () => { + /** + * Minimal inline tool wrapping text with + */ + class FontTool { + public static isInline = true; + + /** + * + */ + public static get sanitize(): SanitizerConfig { + return { + span: { + class: true, + 'data-font-setting': true, + }, + } as unknown as SanitizerConfig; + } + + /** render is required for correct implementation of the InlineTool class */ + public render(): HTMLElement { + return document.createElement('button'); + } + + /** surround is required for correct implementation of the InlineTool class */ + // eslint-disable-next-line @typescript-eslint/no-empty-function + public surround(): void { } + + /** checkState is required for correct implementation of the InlineTool class */ + public checkState(): boolean { + return false; + } + } + + /** + * Minimal inline tool wrapping text with + */ + class FontSizeTool extends FontTool { + /** + * + */ + public static get sanitize(): SanitizerConfig { + return { + span: { + class: true, + 'data-font-size-setting': true, + }, + } as unknown as SanitizerConfig; + } + } + + const text = 'Plain font and ' + + 'size and ' + + 'both'; + + cy.createEditor({ + tools: { + font: FontTool, + fontSize: FontSizeTool, + }, + data: { + blocks: [ { + type: 'paragraph', + data: { text }, + } ], + }, + }) + .then(async editor => { + cy.wrap(await editor.save()) + .then((output) => { + expect(output.blocks[0].data.text).to.eq(text); + }); + }); + }); + it('should sanitize unwanted html on blocks merging', function () { cy.createEditor({ data: { diff --git a/test/cypress/tests/utils/sanitizer.cy.ts b/test/cypress/tests/utils/sanitizer.cy.ts new file mode 100644 index 000000000..71c0d5da9 --- /dev/null +++ b/test/cypress/tests/utils/sanitizer.cy.ts @@ -0,0 +1,126 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import { clean, mergeSanitizerConfigs } from '../../../../src/components/utils/sanitizer'; +import type { SanitizerConfig } from '../../../../types'; + +describe('Sanitizer utils', () => { + context('mergeSanitizerConfigs()', () => { + it('should keep rules for different tags from all configs', () => { + const merged = mergeSanitizerConfigs( + { b: true } as SanitizerConfig, + { i: {} } as SanitizerConfig + ); + + expect(merged).to.be.deep.eq({ + b: true, + i: {}, + }); + }); + + it('should union attributes when two configs describe the same tag', () => { + const merged = mergeSanitizerConfigs( + { span: { class: true, + 'data-font-setting': true } } as SanitizerConfig, + { span: { class: true, + 'data-font-size-setting': true } } as SanitizerConfig + ); + + expect(merged).to.be.deep.eq({ + span: { + class: true, + 'data-font-setting': true, + 'data-font-size-setting': true, + }, + }); + }); + + it('should not mutate passed configs', () => { + const first = { span: { 'data-a': true } } as SanitizerConfig; + const second = { span: { 'data-b': true } } as SanitizerConfig; + + mergeSanitizerConfigs(first, second); + + expect(first).to.be.deep.eq({ span: { 'data-a': true } }); + expect(second).to.be.deep.eq({ span: { 'data-b': true } }); + }); + + it('should resolve "true" over an attributes object for the same tag', () => { + expect(mergeSanitizerConfigs({ a: true } as SanitizerConfig, { a: { href: true } } as SanitizerConfig)) + .to.be.deep.eq({ a: true }); + expect(mergeSanitizerConfigs({ a: { href: true } } as SanitizerConfig, { a: true } as SanitizerConfig)) + .to.be.deep.eq({ a: true }); + }); + + it('should let an allowing rule win over "false" for the same tag', () => { + expect(mergeSanitizerConfigs({ a: false } as SanitizerConfig, { a: { href: true } } as SanitizerConfig)) + .to.be.deep.eq({ a: { href: true } }); + expect(mergeSanitizerConfigs({ a: { href: true } } as SanitizerConfig, { a: false } as SanitizerConfig)) + .to.be.deep.eq({ a: { href: true } }); + expect(mergeSanitizerConfigs({ a: false } as SanitizerConfig, { a: false } as SanitizerConfig)) + .to.be.deep.eq({ a: false }); + }); + + it('should resolve "true" over "false" for the same attribute', () => { + const merged = mergeSanitizerConfigs( + { span: { class: false } } as SanitizerConfig, + { span: { class: true } } as SanitizerConfig + ); + + expect(merged).to.be.deep.eq({ span: { class: true } }); + }); + + it('should combine different exact attribute values into a function that accepts either', () => { + const merged = mergeSanitizerConfigs( + { a: { target: '_blank' } } as SanitizerConfig, + { a: { target: '_self' } } as SanitizerConfig + ); + + expect(clean('x', merged)).to.eq('x'); + expect(clean('x', merged)).to.eq('x'); + expect(clean('x', merged)).to.eq('x'); + }); + + it('should combine a function rule with an object rule for the same tag', () => { + const merged = mergeSanitizerConfigs( + { + span: (el: Element) => { + return el.classList.contains('font') ? { class: true, + 'data-font': true } : {}; + }, + } as unknown as SanitizerConfig, + { span: { 'data-size': true } } as SanitizerConfig + ); + + expect(merged.span).to.be.a('function'); + + expect(clean('t', merged)) + .to.eq('t'); + expect(clean('t', merged)) + .to.eq('t'); + }); + + it('should combine two function rules for the same tag', () => { + const merged = mergeSanitizerConfigs( + { u: (el: Element) => el.textContent === 'first' } as unknown as SanitizerConfig, + { u: (el: Element) => el.textContent === 'second' } as unknown as SanitizerConfig + ); + + expect(clean('first', merged)).to.eq('first'); + expect(clean('second', merged)).to.eq('second'); + expect(clean('third', merged)).to.eq('third'); + }); + + it('should resolve "true" over a function rule for the same tag', () => { + const merged = mergeSanitizerConfigs( + { u: (el: Element) => el.textContent === 'first' } as unknown as SanitizerConfig, + { u: true } as SanitizerConfig + ); + + expect(merged).to.be.deep.eq({ u: true }); + }); + + it('should ignore undefined configs', () => { + expect(mergeSanitizerConfigs(undefined, { b: true } as SanitizerConfig, undefined)) + .to.be.deep.eq({ b: true }); + }); + }); +});