Fix for race condition while uploading multiple files with uploadimage plugin

CHANGES.md

@@ -31,6 +31,7 @@ Fixed Issues:
 * [#1057](https://github.com/ckeditor/ckeditor-dev/issues/1057): Fixed: The [Notification](https://ckeditor.com/addon/notification) plugin overwrites Web Notifications API due to leakage to the global scope.
 * [#1068](https://github.com/ckeditor/ckeditor-dev/issues/1068): Fixed: Upload widget paste listener ignores changes to the [`uploadWidgetDefinition`](https://docs.ckeditor.com/ckeditor4/docs/#!/api/CKEDITOR.fileTools.uploadWidgetDefinition).
 * [#921](https://github.com/ckeditor/ckeditor-dev/issues/921): Fixed: [Edge] CKEditor erroneously perceives internal copy and paste as type "external".
+* [#1213](https://github.com/ckeditor/ckeditor-dev/issues/1213): Fixed: Multiple images uploaded using [Upload Image](https://ckeditor.com/cke4/addon/uploadimage) plugin are randomly duplicated or mangled.
 
 API Changes:
 

plugins/uploadimage/plugin.js

@@ -6,6 +6,29 @@
 'use strict';
 
 ( function() {
+	var uniqueNameCounter = 0,
+		// Black rectangle which is shown before image is loaded.
+		loadingImage = 'data:image/gif;base64,R0lGODlhDgAOAIAAAAAAAP///yH5BAAAAAAALAAAAAAOAA4AAAIMhI+py+0Po5y02qsKADs=';
+
+	// Returns number as a string. If a number has 1 digit only it returns it prefixed with an extra 0.
+	function padNumber( input ) {
+		if ( input <= 9 ) {
+			input = '0' + input;
+		}
+
+		return String( input );
+	}
+
+	// Returns an unique image file name.
+	function getUniqueImageFileName( type ) {
+		var date = new Date(),
+			dateParts = [ date.getFullYear(), date.getMonth() + 1, date.getDate(), date.getHours(), date.getMinutes(), date.getSeconds() ];
+
+		uniqueNameCounter += 1;
+
+		return 'image-' + CKEDITOR.tools.array.map( dateParts, padNumber ).join( '' ) + '-' + uniqueNameCounter + '.' + type;
+	}
+
 	CKEDITOR.plugins.add( 'uploadimage', {
 		requires: 'uploadwidget',
 
@@ -89,13 +112,22 @@
 				for ( i = 0; i < imgs.count(); i++ ) {
 					img = imgs.getItem( i );
 
-					// Image have to contain src=data:...
-					var isDataInSrc = img.getAttribute( 'src' ) && img.getAttribute( 'src' ).substring( 0, 5 ) == 'data:',
+					// Assign src once, as it might be a big string, so there's no point in duplicating it all over the place.
+					var imgSrc = img.getAttribute( 'src' ),
+						// Image have to contain src=data:...
+						isDataInSrc = imgSrc && imgSrc.substring( 0, 5 ) == 'data:',
 						isRealObject = img.data( 'cke-realelement' ) === null;
 
 					// We are not uploading images in non-editable blocs and fake objects (https://dev.ckeditor.com/ticket/13003).
 					if ( isDataInSrc && isRealObject && !img.data( 'cke-upload-id' ) && !img.isReadOnly( 1 ) ) {
-						var loader = editor.uploadRepository.create( img.getAttribute( 'src' ) );
+						// Note normally we'd extract this logic into a separate function, but we should not duplicate this string, as it might
+						// be large.
+						var imgFormat = imgSrc.match( /image\/([a-z]+?);/i ),
+							loader;
+
+						imgFormat = ( imgFormat && imgFormat[ 1 ] ) || 'jpg';
+
+						loader = editor.uploadRepository.create( imgSrc, getUniqueImageFileName( imgFormat ) );
 						loader.upload( uploadUrl );
 
 						fileTools.markElement( img, 'uploadimage', loader.id );
@@ -109,11 +141,6 @@
 		}
 	} );
 
-	// jscs:disable maximumLineLength
-	// Black rectangle which is shown before image is loaded.
-	var loadingImage = 'data:image/gif;base64,R0lGODlhDgAOAIAAAAAAAP///yH5BAAAAAAALAAAAAAOAA4AAAIMhI+py+0Po5y02qsKADs=';
-	// jscs:enable maximumLineLength
-
 	/**
 	 * The URL where images should be uploaded.
 	 *

tests/plugins/uploadimage/uploadimage.js

@@ -538,15 +538,41 @@
 
 		'test prevent upload fake elements (https://dev.ckeditor.com/ticket/13003)': function() {
 			var editor = this.editors.inline,
-				createspy = sinon.spy( editor.uploadRepository, 'create' );
+				createSpy = sinon.spy( editor.uploadRepository, 'create' );
 
 			editor.fire( 'paste', {
 				dataValue: '<img src="data:image/gif;base64,aw==" alt="nothing" data-cke-realelement="some" />'
 			} );
 
 			editor.once( 'afterPaste', function() {
 				resume( function() {
-					assert.isTrue( createspy.notCalled );
+					createSpy.restore();
+					assert.isTrue( createSpy.notCalled );
+				} );
+			} );
+
+			wait();
+		},
+
+		'test uploads generate unique names (#1213)': function() {
+			var editor = this.editors.inline,
+				createSpy = sinon.spy( editor.uploadRepository, 'create' );
+
+			editor.fire( 'paste', {
+				dataValue: '<img src="data:image/gif;base64,aw==" alt="gif" />' +
+					'<img src="data:image/gif;base64,aw==" alt="gif" />' +
+					'<img src="data:image/png;base64,aw==" alt="png" />'
+			} );
+
+			editor.once( 'afterPaste', function() {
+				resume( function() {
+					createSpy.restore();
+					assert.areSame( 3, createSpy.callCount, 'create call count' );
+
+					assert.isMatching( /image-\d+-\d+\.gif/, createSpy.args[ 0 ][ 1 ], 'file name passed to first call' );
+					assert.isMatching( /image-\d+-\d+\.gif/, createSpy.args[ 1 ][ 1 ], 'file name passed to second call' );
+					assert.areNotSame( createSpy.args[ 0 ][ 1 ], createSpy.args[ 1 ][ 1 ], 'first and second call names are different' );
+					assert.isMatching( /image-\d+-\d+\.png/, createSpy.args[ 2 ][ 1 ], 'png type is recognized' );
 				} );
 			} );