From ae1ab4607c7f0c5172419e2c87f6314a4a111c40 Mon Sep 17 00:00:00 2001 From: Shaswat Raj Date: Sun, 1 Nov 2026 17:48:27 +0530 Subject: [PATCH 1/2] fix(sanitizer): merge sanitize rules per tag instead of overwriting them When several Inline Tools (or Block Tunes) declare sanitize rules for the same tag, the configs were combined with Object.assign, so the last tool's rule replaced the previous ones. Two tools that both wrap text in with different allowed attributes lost one set of attributes on save. Add mergeSanitizerConfigs() which combines rules per tag: boolean, attribute object and function rules are merged so that an attribute allowed by any contributor stays allowed. Use it in BlockToolAdapter.baseSanitizeConfig and Tools.getAllInlineToolsSanitizeConfig. Closes #2501 Co-Authored-By: Claude Fable 5.1 --- src/components/modules/tools.ts | 12 +- src/components/tools/block.ts | 16 +-- src/components/utils/sanitizer.ts | 133 ++++++++++++++++++++++- test/cypress/tests/sanitisation.cy.ts | 82 +++++++++++++- test/cypress/tests/utils/sanitizer.cy.ts | 126 +++++++++++++++++++++ 5 files changed, 347 insertions(+), 22 deletions(-) create mode 100644 test/cypress/tests/utils/sanitizer.cy.ts 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 }); + }); + }); +}); From e4e887f22e62de2824b2864f42dca10396532734 Mon Sep 17 00:00:00 2001 From: Shaswat Raj Date: Mon, 2 Nov 2026 13:10:58 +0530 Subject: [PATCH 2/2] chore: bump version to 2.32.0-rc.4 and add changelog entry Co-Authored-By: Claude Fable 5.1 --- docs/CHANGELOG.md | 1 + package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) 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",