Merge pull request #917 from ckeditor/t/881

Fix: Improved performance of the `view.Element` inline styles parser. Big property values (like based64 encoded images) should not crash the editor anymore. Closes #881.

src/view/element.js

@@ -685,12 +685,72 @@ export default class Element extends Node {
 // @param {Map.<String, String>} stylesMap Map to insert parsed properties and values.
 // @param {String} stylesString Styles to parse.
 function parseInlineStyles( stylesMap, stylesString ) {
-	const regex = /\s*([^:;\s]+)\s*:\s*([^;]+)\s*(?=;|$)/g;
-	let matchStyle;
+	// `null` if no quote was found in input string or last found quote was a closing quote. See below.
+	let quoteType = null;
+	let propertyNameStart = 0;
+	let propertyValueStart = 0;
+	let propertyName = null;
+
 	stylesMap.clear();
 
-	while ( ( matchStyle = regex.exec( stylesString ) ) !== null ) {
-		stylesMap.set( matchStyle[ 1 ], matchStyle[ 2 ].trim() );
+	// Do not set anything if input string is empty.
+	if ( stylesString === '' ) {
+		return;
+	}
+
+	// Fix inline styles that do not end with `;` so they are compatible with algorithm below.
+	if ( stylesString.charAt( stylesString.length - 1 ) != ';' ) {
+		stylesString = stylesString + ';';
+	}
+
+	// Seek the whole string for "special characters".
+	for ( let i = 0; i < stylesString.length; i++ ) {
+		const char = stylesString.charAt( i );
+
+		if ( quoteType === null ) {
+			// No quote found yet or last found quote was a closing quote.
+			switch ( char ) {
+				case ':':
+					// Most of time colon means that property name just ended.
+					// Sometimes however `:` is found inside property value (for example in background image url).
+					if ( !propertyName ) {
+						// Treat this as end of property only if property name is not already saved.
+						// Save property name.
+						propertyName = stylesString.substr( propertyNameStart, i - propertyNameStart );
+						// Save this point as the start of property value.
+						propertyValueStart = i + 1;
+					}
+
+					break;
+
+				case '"':
+				case '\'':
+					// Opening quote found (this is an opening quote, because `quoteType` is `null`).
+					quoteType = char;
+
+					break;
+
+				case ';':
+					// Property value just ended.
+					// Use previously stored property value start to obtain property value.
+					let propertyValue = stylesString.substr( propertyValueStart, i - propertyValueStart );
+
+					if ( propertyName ) {
+						// Save parsed part.
+						stylesMap.set( propertyName.trim(), propertyValue.trim() );
+					}
+
+					propertyName = null;
+
+					// Save this point as property name start. Property name starts immediately after previous property value ends.
+					propertyNameStart = i + 1;
+
+					break;
+			}
+		} else if ( char === quoteType ) {
+			// If a quote char is found and it is a closing quote, mark this fact by `null`-ing `quoteType`.
+			quoteType = null;
+		}
 	}
 }
 

tests/view/element.js

@@ -7,6 +7,8 @@ import count from '@ckeditor/ckeditor5-utils/src/count';
 import Node from '../../src/view/node';
 import Element from '../../src/view/element';
 
+import encodedImage from './_utils/encodedimage.txt';
+
 describe( 'Element', () => {
 	describe( 'constructor()', () => {
 		it( 'should create element without attributes', () => {
@@ -767,10 +769,12 @@ describe( 'Element', () => {
 		describe( 'getStyleNames', () => {
 			it( 'should return iterator with all style names', () => {
 				const names = [ 'color', 'position' ];
+
 				el.setStyle( {
 					color: 'red',
 					position: 'absolute'
 				} );
+
 				const iterator = el.getStyleNames();
 				let i = 0;
 
@@ -832,6 +836,67 @@ describe( 'Element', () => {
 				expect( el.hasStyle( 'color' ) ).to.be.true;
 			} );
 		} );
+
+		describe( 'styles parsing edge cases and incorrect styles', () => {
+			it( 'should not crash and not add any styles if styles attribute was empty', () => {
+				const element = new Element( 'div', { style: '' } );
+				const styles = Array.from( element.getStyleNames() );
+
+				expect( styles ).to.deep.equal( [] );
+			} );
+
+			it( 'should be able to parse big styles definition', () => {
+				expect( () => {
+					new Element( 'div', { style: `background-image:url('data:image/jpeg;base64,${ encodedImage }')` } );
+				} ).not.to.throw();
+			} );
+
+			it( 'should work with both types of quotes and ignore values inside quotes', () => {
+				let element;
+
+				element = new Element( 'div', { style: 'background-image:url("im;color:g.jpg")' } );
+				expect( element.getStyle( 'background-image' ) ).to.equal( 'url("im;color:g.jpg")' );
+
+				element = new Element( 'div', { style: 'background-image:url(\'im;color:g.jpg\')' } );
+				expect( element.getStyle( 'background-image' ) ).to.equal( 'url(\'im;color:g.jpg\')' );
+			} );
+
+			it( 'should not be confused by whitespaces', () => {
+				const element = new Element( 'div', { style: '\ncolor:\n red ' } );
+
+				expect( element.getStyle( 'color' ) ).to.equal( 'red' );
+			} );
+
+			it( 'should not be confused by duplicated semicolon', () => {
+				const element = new Element( 'div', { style: 'color: red;; display: inline' } );
+
+				expect( element.getStyle( 'color' ) ).to.equal( 'red' );
+				expect( element.getStyle( 'display' ) ).to.equal( 'inline' );
+			} );
+
+			it( 'should not throw when value is missing', () => {
+				const element = new Element( 'div', { style: 'color:; display: inline' } );
+
+				expect( element.getStyle( 'color' ) ).to.equal( '' );
+				expect( element.getStyle( 'display' ) ).to.equal( 'inline' );
+			} );
+
+			it( 'should not throw when colon is duplicated', () => {
+				const element = new Element( 'div', { style: 'color:: red; display: inline' } );
+
+				// The result makes no sense, but here we just check that the algorithm doesn't crash.
+				expect( element.getStyle( 'color' ) ).to.equal( ': red' );
+				expect( element.getStyle( 'display' ) ).to.equal( 'inline' );
+			} );
+
+			it( 'should not throw when random stuff passed', () => {
+				const element = new Element( 'div', { style: 'color: red;:; ;;" ":  display: inline; \'aaa;:' } );
+
+				// The result makes no sense, but here we just check that the algorithm doesn't crash.
+				expect( element.getStyle( 'color' ) ).to.equal( 'red' );
+				expect( element.getStyle( 'display' ) ).to.be.undefined;
+			} );
+		} );
 	} );
 
 	describe( 'findAncestor', () => {