diff --git a/packages/ckeditor5-engine/src/view/domconverter.ts b/packages/ckeditor5-engine/src/view/domconverter.ts index fbaaf7fdfb1..ee9c9305dc3 100644 --- a/packages/ckeditor5-engine/src/view/domconverter.ts +++ b/packages/ckeditor5-engine/src/view/domconverter.ts @@ -30,6 +30,7 @@ import { getAncestors, isText, isComment, + isValidAttributeName, first } from '@ckeditor/ckeditor5-utils'; @@ -453,6 +454,17 @@ export default class DomConverter { logWarning( 'domconverter-unsafe-attribute-detected', { domElement, key, value } ); } + if ( !isValidAttributeName( key ) ) { + /** + * Invalid attribute name was ignored during rendering. + * + * @error domconverter-invalid-attribute-detected + */ + logWarning( 'domconverter-invalid-attribute-detected', { domElement, key, value } ); + + return; + } + // The old value was safe but the new value is unsafe. if ( domElement.hasAttribute( key ) && !shouldRenderAttribute ) { domElement.removeAttribute( key ); diff --git a/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js b/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js index 70f32334690..e8233c8a768 100644 --- a/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js +++ b/packages/ckeditor5-engine/tests/view/domconverter/domconverter.js @@ -784,7 +784,7 @@ describe( 'DomConverter', () => { } ); warnStub = testUtils.sinon.stub( console, 'warn' ) - .withArgs( sinon.match( /^domconverter-unsafe-attribute-detected/ ) ) + .withArgs( sinon.match( /^domconverter-unsafe-attribute-detected|domconverter-invalid-attribute-detected/ ) ) .callsFake( () => {} ); console.warn.callThrough(); @@ -880,6 +880,30 @@ describe( 'DomConverter', () => { ); } ); + it( 'should not render the attribute with invalid name', () => { + const domElement = document.createElement( 'p' ); + + converter.setDomElementAttribute( domElement, '200', 'foo' ); + expect( domElement.outerHTML ).to.equal( '

' ); + } ); + + it( 'should warn when the attribute has invalid name', () => { + const domElement = document.createElement( 'p' ); + + converter.setDomElementAttribute( domElement, '200', 'foo' ); + + sinon.assert.calledOnce( warnStub ); + sinon.assert.calledWithExactly( warnStub, + sinon.match( /^domconverter-invalid-attribute-detected/ ), + { + domElement, + key: '200', + value: 'foo' + }, + sinon.match.string // Link to the documentation + ); + } ); + it( 'should set src attribute for SVG on img element', () => { const domElement = document.createElement( 'img' ); diff --git a/packages/ckeditor5-html-support/src/datafilter.ts b/packages/ckeditor5-html-support/src/datafilter.ts index 41a0edcccd2..c9c10a38a00 100644 --- a/packages/ckeditor5-html-support/src/datafilter.ts +++ b/packages/ckeditor5-html-support/src/datafilter.ts @@ -7,9 +7,8 @@ * @module html-support/datafilter */ -/* globals document */ - import { Plugin, type Editor } from 'ckeditor5/src/core'; + import { Matcher, type MatcherPattern, @@ -18,8 +17,15 @@ import { type MatchResult, type ViewConsumable } from 'ckeditor5/src/engine'; -import { priorities, CKEditorError } from 'ckeditor5/src/utils'; + +import { + CKEditorError, + priorities, + isValidAttributeName +} from 'ckeditor5/src/utils'; + import { Widget } from 'ckeditor5/src/widget'; + import { viewToModelObjectConverter, toObjectWidgetConverter, @@ -31,12 +37,14 @@ import { viewToModelBlockAttributeConverter, modelToViewBlockAttributeConverter } from './converters'; + import { default as DataSchema, type DataSchemaBlockElementDefinition, type DataSchemaDefinition, type DataSchemaInlineElementDefinition } from './dataschema'; + import type { GHSViewAttributes } from './utils'; import { isPlainObject, pull as removeItemFromArray } from 'lodash-es'; @@ -791,16 +799,3 @@ function splitRules( rules: MatcherPatternWithName ): Array { return splittedRules; } - -/** - * Returns true if name is valid for a DOM attribute name. - */ -function isValidAttributeName( name: string ): boolean { - try { - document.createAttribute( name ); - } catch ( error ) { - return false; - } - - return true; -} diff --git a/packages/ckeditor5-html-support/tests/integrations/customelement.js b/packages/ckeditor5-html-support/tests/integrations/customelement.js index 5e9e8621ed1..ce3e83fe5f9 100644 --- a/packages/ckeditor5-html-support/tests/integrations/customelement.js +++ b/packages/ckeditor5-html-support/tests/integrations/customelement.js @@ -9,17 +9,20 @@ import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock'; import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; import { INLINE_FILLER } from '@ckeditor/ckeditor5-engine/src/view/filler'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import GeneralHtmlSupport from '../../src/generalhtmlsupport'; import { getModelDataWithAttributes } from '../_utils/utils'; -/* global document */ +/* global document, console */ describe( 'CustomElementSupport', () => { let editor, model, editorElement, dataFilter; const excludeAttributes = [ 'htmlContent', 'htmlElementName' ]; + testUtils.createSinonSandbox(); + beforeEach( () => { editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); @@ -275,6 +278,57 @@ describe( 'CustomElementSupport', () => { expect( editor.getData() ).to.equal( 'bar' ); } ); + it( 'should allow attributes without `data-` prefix', () => { + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { attributes: { 'foo': /.*/ } } ); + + editor.setData( 'baz' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'baz"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + attributes: { + 'foo': 'bar' + } + } + } + } ); + + expect( editor.getData() ).to.equal( 'baz' ); + } ); + + it( 'should ignore attributes with invalid name', () => { + const consoleWarnStub = sinon.stub( console, 'warn' ); + + dataFilter.allowElement( /.*/ ); + dataFilter.allowAttributes( { attributes: /.*/ } ); + + editor.setData( 'baz' ); + + expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( { + data: 'baz"' + + ' htmlElementName="custom-foo-element">', + attributes: { + 1: { + attributes: { + 'data-foo': 'bar' + } + } + } + } ); + + expect( editor.getData() ).to.equal( 'baz' ); + + expect( consoleWarnStub.calledOnce ).to.equal( true ); + expect( consoleWarnStub.firstCall.args[ 0 ] ).to.match( /domconverter-invalid-attribute-detected/ ); + } ); + it( 'should allow attributes (classes)', () => { dataFilter.allowElement( /.*/ ); dataFilter.allowAttributes( { classes: 'foo' } ); diff --git a/packages/ckeditor5-utils/src/dom/isvalidattributename.ts b/packages/ckeditor5-utils/src/dom/isvalidattributename.ts new file mode 100644 index 00000000000..471c278305f --- /dev/null +++ b/packages/ckeditor5-utils/src/dom/isvalidattributename.ts @@ -0,0 +1,25 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/** + * @module utils/dom/isvalidattributename + */ + +import global from './global'; + +/** + * Checks if the given attribute name is valid in terms of HTML. + * + * @param name Attribute name. + */ +export default function isValidAttributeName( name: string ): boolean { + try { + global.document.createAttribute( name ); + } catch ( error ) { + return false; + } + + return true; +} diff --git a/packages/ckeditor5-utils/src/index.ts b/packages/ckeditor5-utils/src/index.ts index 384b0322e1b..c2fb71dd9e2 100644 --- a/packages/ckeditor5-utils/src/index.ts +++ b/packages/ckeditor5-utils/src/index.ts @@ -62,6 +62,7 @@ export { default as insertAt } from './dom/insertat'; export { default as isComment } from './dom/iscomment'; export { default as isNode } from './dom/isnode'; export { default as isRange } from './dom/isrange'; +export { default as isValidAttributeName } from './dom/isvalidattributename'; export { default as isVisible } from './dom/isvisible'; export { getOptimalPosition, type Options as PositionOptions, type PositioningFunction } from './dom/position'; export { default as remove } from './dom/remove'; diff --git a/packages/ckeditor5-utils/tests/dom/isvalidattributename.js b/packages/ckeditor5-utils/tests/dom/isvalidattributename.js new file mode 100644 index 00000000000..722b2b0fafa --- /dev/null +++ b/packages/ckeditor5-utils/tests/dom/isvalidattributename.js @@ -0,0 +1,36 @@ +/** + * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import isValidAttributeName from '../../src/dom/isvalidattributename'; + +describe( 'isValidAttributeName', () => { + const validTestCases = [ + 'src', + 'data-foo', + 'href', + 'class', + 'style', + 'id', + 'name' + ]; + + for ( const name of validTestCases ) { + it( `should return true for '${ name }'`, () => { + expect( isValidAttributeName( name ) ).to.be.true; + } ); + } + + const invalidTestCases = [ + '200', + '-data', + '7abc' + ]; + + for ( const name of invalidTestCases ) { + it( `should return false for '${ name }'`, () => { + expect( isValidAttributeName( name ) ).to.be.false; + } ); + } +} );