Преглед на файлове

Created HtmlWriter and DataProcessor interfaces, fixed and added test against XSS attacks for HtmlDataProcessor.

Szymon Kupś преди 9 години
родител
ревизия
952910b399

+ 3 - 1
packages/ckeditor5-engine/src/dataprocessor/basichtmlwriter.js

@@ -11,6 +11,7 @@
  * from DocumentFragment to HTML string.
  *
  * @class dataProcessor.BasicHtmlWriter
+ * @implements dataProcessor.HtmlWriter
  */
 export default class BasicHtmlWriter {
 	/**
@@ -20,7 +21,8 @@ export default class BasicHtmlWriter {
 	 * @returns {String}
 	 */
 	getHtml( fragment ) {
-		const container = document.createElement( 'div' );
+		const doc = document.implementation.createHTMLDocument( '' );
+		const container = doc.createElement( 'div' );
 		container.appendChild( fragment );
 
 		return container.innerHTML;

+ 18 - 39
packages/ckeditor5-engine/src/dataprocessor/dataprocessor.js

@@ -3,47 +3,26 @@
  * For licensing, see LICENSE.md.
  */
 
-'use strict';
-
-import Config from '../config.js';
-
 /**
- * Abstract base DataProcessor class. This class should be extended by actual DataProcessor implementations.
+ * DataProcessor interface. This should be implemented by actual DataProcessors.
+ * Each data processor implements a certain format of the data. E.g. `MarkdownDataProcessor` will convert the data
+ * (Markdown string) to a DocumentFragment and back.
  *
- * @abstract
- * @class dataProcessor.DataProcessor
+ * @interface dataProcessor.DataProcessor
  */
-export default class DataProcessor {
-	/**
-	 * Creates a new instance of DataProcessor class.
-	 *
-	 *
-	 * @param {Object} config DataProcessor's configuration.
-	 * @constructor
-	 */
-	constructor( config ) {
-		/**
-		 * Holds configurations specific to this DataProcessor instance.
-		 *
-		 * @readonly
-		 * @property {Config}
-		 */
-		this.config = new Config( config );
-	}
 
-	/**
-	 * Converts DocumentFragment to data supported by DataProcessor.
-	 *
-	 * @method toData
-	 * @param {DocumentFragment} fragment Document fragment to be processed.
-	 * @returns {*} Data supported by DataProcessor implementation.
-	 */
+/**
+ * Converts a DocumentFragment to the data.
+ *
+ * @method toData
+ * @param {DocumentFragment} fragment DocumentFragment to be processed.
+ * @returns {*}
+ */
 
-	/**
-	 * Converts data supported by DataProcessor to DocumentFragment.
-	 *
-	 * @method toDom
-	 * @param {*} data Data to be processed.
-	 * @returns {DocumentFragment} DocumentFragment
-	 */
-}
+/**
+ * Converts the data to a DocumentFragment.
+ *
+ * @method toDom
+ * @param {*} data Data to be processed.
+ * @returns {DocumentFragment}
+ */

+ 4 - 9
packages/ckeditor5-engine/src/dataprocessor/htmldataprocessor.js

@@ -5,7 +5,6 @@
 
 'use strict';
 
-import DataProcessor from './dataprocessor.js';
 import BasicHtmlWriter from './basichtmlwriter.js';
 
 /**
@@ -13,19 +12,15 @@ import BasicHtmlWriter from './basichtmlwriter.js';
  * This data processor implementation uses HTML as input/output data.
  *
  * @class dataProcessor.HtmlDataProcessor
- * @extends dataProcessor.DataProcessor
+ * @implements dataProcessor.DataProcessor
  */
-export default class HtmlDataProcessor extends DataProcessor {
+export default class HtmlDataProcessor {
 	/**
 	 * Creates a new instance of the HtmlDataProcessor class.
 	 *
-	 *
-	 * @param {Object} config HtmlDataProcessor's configuration.
 	 * @constructor
 	 */
-	constructor( config ) {
-		super( config );
-
+	constructor() {
 		/**
 		 * DOMParser instance used to parse HTML string to HTMLDocument.
 		 *
@@ -66,7 +61,7 @@ export default class HtmlDataProcessor extends DataProcessor {
 		const nodes = document.body.childNodes;
 
 		while ( nodes.length > 0 ) {
-			fragment.appendChild( nodes [0] );
+			fragment.appendChild( nodes[ 0 ] );
 		}
 
 		return fragment;

+ 18 - 0
packages/ckeditor5-engine/src/dataprocessor/htmlwriter.js

@@ -0,0 +1,18 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * HTML writer interface.
+ *
+ * @interface dataProcessor.HtmlWriter
+ */
+
+/**
+ * Returns HTML string created from DocumentFragment.
+ *
+ * @method getHtml
+ * @param {DocumentFragment} fragment
+ * @returns {String}
+ */

+ 15 - 0
packages/ckeditor5-engine/tests/dataprocessor/_utils/xsstemplates.js

@@ -0,0 +1,15 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+'use strict';
+
+export default {
+	'img onerror': '<p><img onerror="%xss%" src="produce404" /></p>',
+	'video onerror': '<p><video onerror="%xss%">foo</video></p>',
+	'video onerror + src': '<p><video onerror="%xss%" src="produce404">foo</video></p>',
+	'video with source onerror': '<p><video><source onerror="%xss%" /></video></p>',
+	'video with source onerror 2': '<p><video><source onerror="%xss%" src="produce404" /></video></p>',
+	'script': '<script>%xss%<\/script>'
+};

+ 55 - 0
packages/ckeditor5-engine/tests/dataprocessor/basichtmlwriter.js

@@ -0,0 +1,55 @@
+/**
+ * @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+'use strict';
+
+import BasicHtmlWriter from '/ckeditor5/core/dataprocessor/basichtmlwriter.js';
+
+describe( 'BasicHtmlWriter', () => {
+	const basicHtmlWriter = new BasicHtmlWriter();
+
+	it( 'should return empty string when empty DocumentFragment is passed' , () => {
+		const data = basicHtmlWriter.getHtml( document.createDocumentFragment() );
+		expect( data ).to.equal( '' );
+	} );
+
+	it( 'should create text from single text node', () => {
+		const text = 'foo bar';
+		const fragment = document.createDocumentFragment();
+		const textNode = document.createTextNode( text );
+		fragment.appendChild( textNode );
+
+		const data = basicHtmlWriter.getHtml( fragment );
+		expect( data ).to.equal( text );
+	} );
+
+	it( 'should return correct HTML from fragment with paragraph', () => {
+		const fragment = document.createDocumentFragment();
+		const paragraph = document.createElement( 'p' );
+		paragraph.textContent = 'foo bar';
+		fragment.appendChild( paragraph );
+
+		const data = basicHtmlWriter.getHtml( fragment );
+		expect( data ).to.equal( '<p>foo bar</p>' );
+	} );
+
+	it( 'should return correct HTML from fragment with multiple child nodes', () => {
+		const fragment = document.createDocumentFragment();
+		const text = document.createTextNode( 'foo bar' );
+		const paragraph = document.createElement( 'p' );
+		const div = document.createElement( 'div' );
+
+		paragraph.textContent = 'foo';
+		div.textContent = 'bar';
+
+		fragment.appendChild( text );
+		fragment.appendChild( paragraph );
+		fragment.appendChild( div );
+
+		const data = basicHtmlWriter.getHtml( fragment );
+
+		expect( data ).to.equal( 'foo bar<p>foo</p><div>bar</div>' );
+	} );
+} );

+ 0 - 34
packages/ckeditor5-engine/tests/dataprocessor/dataprocessor.js

@@ -1,34 +0,0 @@
-/**
- * @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-'use strict';
-
-import DataProcessor from '/ckeditor5/core/dataprocessor/dataprocessor.js';
-
-describe( 'DataProcessor', () => {
-	const config = {
-		option: 'test'
-	};
-	const dataProcessor = new DataProcessor( config );
-
-	describe( 'constructor', () => {
-		it( 'should set the `config` property', () => {
-			expect( dataProcessor ).to.have.property( 'config' );
-			expect( dataProcessor.config.get( 'option' ) ).to.equal( 'test' );
-		} );
-	} );
-
-	describe( 'toData', () => {
-		it( 'should be defined', () => {
-			expect( dataProcessor.toData ).to.be.function;
-		} );
-	} );
-
-	describe( 'toDom', () => {
-		it( 'should be defined', () => {
-			expect( dataProcessor.toDom ).to.be.function;
-		} );
-	} );
-} );

+ 19 - 56
packages/ckeditor5-engine/tests/dataprocessor/htmldataprocessor.js

@@ -6,6 +6,7 @@
 'use strict';
 
 import HtmlDataProcessor from '/ckeditor5/core/dataprocessor/htmldataprocessor.js';
+import xssTemplates from '_utils/xsstemplates.js';
 
 describe( 'HtmlDataProcessor', () => {
 	const dataProcessor = new HtmlDataProcessor();
@@ -51,71 +52,33 @@ describe( 'HtmlDataProcessor', () => {
 			expect( fragment.childNodes[ 2 ].textContent ).to.equal( ' text' );
 		} );
 
-		it( 'should parse element attributes', () => {
-			const fragment = dataProcessor.toDom( '<p class="paragraph" data-id="12"></p>' );
-			expect( fragment.childNodes.length ).to.equal( 1 );
-			const childNode = fragment.childNodes[ 0 ];
+		// Test against XSS attacks.
+		for ( const name in xssTemplates ) {
+			const input = xssTemplates[ name ].replace( /%xss%/g, 'testXss()' );
 
-			expect( childNode.attributes.length ).to.equal( 2 );
-			expect( childNode.getAttribute( 'class' ) ).to.equal( 'paragraph' );
-			expect( childNode.getAttribute( 'data-id' ) ).to.equal( '12' );
-		} );
+			it( 'should prevent XSS attacks: ' + name, ( done ) => {
+				window.testXss = sinon.spy();
+				dataProcessor.toDom( input );
+
+				setTimeout( () => {
+					sinon.assert.notCalled( window.testXss );
+					done();
+				}, 10 );
+			} );
+		}
 	} );
 
 	describe( 'toData', () => {
-		it( 'should return empty string when empty DocumentFragment is passed' , () => {
-			const data = dataProcessor.toData( document.createDocumentFragment() );
-			expect( data ).to.equal( '' );
-		} );
-
-		it( 'should create text from single text node', () => {
-			const text = 'foo bar';
-			const fragment = document.createDocumentFragment();
-			const textNode = document.createTextNode( text );
-			fragment.appendChild( textNode );
-
-			const data = dataProcessor.toData( fragment );
-			expect( data ).to.equal( text );
-		} );
-
-		it( 'should return correct HTML from fragment with paragraph', () => {
-			const fragment = document.createDocumentFragment();
-			const paragraph = document.createElement( 'p' );
-			paragraph.textContent = 'foo bar';
-			fragment.appendChild( paragraph );
+		it( 'should use HtmlWriter', () => {
+			const spy = sinon.spy( dataProcessor._htmlWriter, 'getHtml' );
 
-			const data = dataProcessor.toData( fragment );
-			expect( data ).to.equal( '<p>foo bar</p>' );
-		} );
-
-		it( 'should return correct HTML from fragment with multiple child nodes', () => {
-			const fragment = document.createDocumentFragment();
-			const text = document.createTextNode( 'foo bar' );
-			const paragraph = document.createElement( 'p' );
-			const div = document.createElement( 'div' );
-
-			paragraph.textContent = 'foo';
-			div.textContent = 'bar';
-
-			fragment.appendChild( text );
-			fragment.appendChild( paragraph );
-			fragment.appendChild( div );
-
-			const data = dataProcessor.toData( fragment );
-
-			expect( data ).to.equal( 'foo bar<p>foo</p><div>bar</div>' );
-		} );
-
-		it( 'should return HTML with attributes', () => {
 			const fragment = document.createDocumentFragment();
 			const paragraph = document.createElement( 'p' );
-			paragraph.setAttribute( 'class', 'paragraph' );
-			paragraph.setAttribute( 'data-id', '12' );
 			fragment.appendChild( paragraph );
+			dataProcessor.toData( fragment );
 
-			const data = dataProcessor.toData( fragment );
-
-			expect( data ).to.equal( '<p class="paragraph" data-id="12"></p>' );
+			spy.restore();
+			sinon.assert.calledWithExactly( spy, fragment );
 		} );
 	} );
 } );