GHS should ignore invalid attribute.

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 );

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( '<p></p>' );
+		} );
+
+		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' );
 

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<MatcherPattern> {
 
 	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;
-}

packages/ckeditor5-html-support/tests/integrations/customelement.js

@@ -275,6 +275,52 @@ describe( 'CustomElementSupport', () => {
 			expect( editor.getData() ).to.equal( '<custom-foo-element data-foo="foo">bar</custom-foo-element>' );
 		} );
 
+		it( 'should allow attributes without `data-` prefix', () => {
+			dataFilter.allowElement( /.*/ );
+			dataFilter.allowAttributes( { attributes: { 'foo': /.*/ } } );
+
+			editor.setData( '<custom-foo-element foo="bar">baz</custom-foo-element>' );
+
+			expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( {
+				data: '<htmlCustomElement' +
+					' htmlAttributes="(1)"' +
+					' htmlContent="<custom-foo-element foo="bar">baz</custom-foo-element>"' +
+					' htmlElementName="custom-foo-element"></htmlCustomElement>',
+				attributes: {
+					1: {
+						attributes: {
+							'foo': 'bar'
+						}
+					}
+				}
+			} );
+
+			expect( editor.getData() ).to.equal( '<custom-foo-element foo="bar">baz</custom-foo-element>' );
+		} );
+
+		it( 'should ignore attributes with invalid name', () => {
+			dataFilter.allowElement( /.*/ );
+			dataFilter.allowAttributes( { attributes: /.*/ } );
+
+			editor.setData( '<custom-foo-element 200-abc="invalid" data-foo="bar">baz</custom-foo-element>' );
+
+			expect( getModelDataWithAttributes( model, { withoutSelection: true, excludeAttributes } ) ).to.deep.equal( {
+				data: '<htmlCustomElement' +
+					' htmlAttributes="(1)"' +
+					' htmlContent="<custom-foo-element data-foo="bar">baz</custom-foo-element>"' +
+					' htmlElementName="custom-foo-element"></htmlCustomElement>',
+				attributes: {
+					1: {
+						attributes: {
+							'data-foo': 'bar'
+						}
+					}
+				}
+			} );
+
+			expect( editor.getData() ).to.equal( '<custom-foo-element data-foo="bar">baz</custom-foo-element>' );
+		} );
+
 		it( 'should allow attributes (classes)', () => {
 			dataFilter.allowElement( /.*/ );
 			dataFilter.allowAttributes( { classes: 'foo' } );

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;
+}

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';

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;
+		} );
+	}
+} );