From 24eeb274f55b7206f8d295ed31ebff7da299ed07 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 23 Nov 2017 13:03:00 +0100 Subject: [PATCH 1/6] Added a workaround for #1213 so that the backend is not spammed with files having the same name. --- plugins/uploadimage/plugin.js | 43 ++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/plugins/uploadimage/plugin.js b/plugins/uploadimage/plugin.js index be43955b4cc..3d5895f6841 100644 --- a/plugins/uploadimage/plugin.js +++ b/plugins/uploadimage/plugin.js @@ -6,6 +6,29 @@ 'use strict'; ( function() { + var counter = 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 padDate( 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() ]; + + counter += 1; + + return 'image-' + CKEDITOR.tools.array.map( dateParts, padDate ).join( '' ) + '-' + counter + '.' + 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. * From 078832ea3ccfc74697bbce58d1d7e9a55b26378d Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 23 Nov 2017 14:47:34 +0100 Subject: [PATCH 2/6] Tests: added unit tests ensuring uploadimage generates unique file names. --- tests/plugins/uploadimage/uploadimage.js | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/plugins/uploadimage/uploadimage.js b/tests/plugins/uploadimage/uploadimage.js index 1033fbbeb5b..b77cb0ec36a 100644 --- a/tests/plugins/uploadimage/uploadimage.js +++ b/tests/plugins/uploadimage/uploadimage.js @@ -68,6 +68,8 @@ this.responseData = {}; }; + + // sinon.spy( CKEDITOR.fileTools.fileLoader.prototype, 'upload' ); }, setUp: function() { @@ -550,6 +552,30 @@ } ); } ); + wait(); + }, + + 'test uploads generate unique names (#1213)': function() { + var editor = this.editors.inline, + createSpy = sinon.spy( editor.uploadRepository, 'create' ); + + editor.fire( 'paste', { + dataValue: 'gif' + + 'gif' + + 'png' + } ); + + editor.once( 'afterPaste', function() { + resume( function() { + 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' ); + } ); + } ); + wait(); } } ); From 342b3a4d8ff60dcc5c1632dae43a21637ffe099d Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 23 Nov 2017 14:50:40 +0100 Subject: [PATCH 3/6] Refactoring: adjusted variable names. --- plugins/uploadimage/plugin.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/uploadimage/plugin.js b/plugins/uploadimage/plugin.js index 3d5895f6841..0571382542b 100644 --- a/plugins/uploadimage/plugin.js +++ b/plugins/uploadimage/plugin.js @@ -6,12 +6,12 @@ 'use strict'; ( function() { - var counter = 0, + 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 padDate( input ) { + function padNumber( input ) { if ( input <= 9 ) { input = '0' + input; } @@ -24,9 +24,9 @@ var date = new Date(), dateParts = [ date.getFullYear(), date.getMonth() + 1, date.getDate(), date.getHours(), date.getMinutes(), date.getSeconds() ]; - counter += 1; + uniqueNameCounter += 1; - return 'image-' + CKEDITOR.tools.array.map( dateParts, padDate ).join( '' ) + '-' + counter + '.' + type; + return 'image-' + CKEDITOR.tools.array.map( dateParts, padNumber ).join( '' ) + '-' + uniqueNameCounter + '.' + type; } CKEDITOR.plugins.add( 'uploadimage', { From 3d2d7a3f503ae076e997416c81748d3894e39091 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 23 Nov 2017 17:10:36 +0100 Subject: [PATCH 4/6] Fixed a case where create method would be wrapped by a spy multiple times. Also removed commented code. --- tests/plugins/uploadimage/uploadimage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/plugins/uploadimage/uploadimage.js b/tests/plugins/uploadimage/uploadimage.js index b77cb0ec36a..a4e54484f4f 100644 --- a/tests/plugins/uploadimage/uploadimage.js +++ b/tests/plugins/uploadimage/uploadimage.js @@ -68,8 +68,6 @@ this.responseData = {}; }; - - // sinon.spy( CKEDITOR.fileTools.fileLoader.prototype, 'upload' ); }, setUp: function() { @@ -548,6 +546,7 @@ editor.once( 'afterPaste', function() { resume( function() { + createspy.restore(); assert.isTrue( createspy.notCalled ); } ); } ); @@ -567,6 +566,7 @@ 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' ); From 9ae12d7c67ff5837203bfd7ac9078a6d369d9649 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 23 Nov 2017 17:11:08 +0100 Subject: [PATCH 5/6] Tests: adjusted variable name. --- tests/plugins/uploadimage/uploadimage.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/plugins/uploadimage/uploadimage.js b/tests/plugins/uploadimage/uploadimage.js index a4e54484f4f..c487ea05203 100644 --- a/tests/plugins/uploadimage/uploadimage.js +++ b/tests/plugins/uploadimage/uploadimage.js @@ -538,7 +538,7 @@ '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: 'nothing' @@ -546,8 +546,8 @@ editor.once( 'afterPaste', function() { resume( function() { - createspy.restore(); - assert.isTrue( createspy.notCalled ); + createSpy.restore(); + assert.isTrue( createSpy.notCalled ); } ); } ); From ee7bf30a374599c550e00d4b21eaa1a37236cc3b Mon Sep 17 00:00:00 2001 From: Tomasz Jakut Date: Thu, 23 Nov 2017 18:11:01 +0100 Subject: [PATCH 6/6] Updated changelog [ci skip]. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index d657d5dceab..83b360d3bde 100644 --- a/CHANGES.md +++ b/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: