From 8737a482c4c1dc5c9896a52f4a4431f9ec51e74e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Fri, 18 Dec 2020 13:57:33 +0100 Subject: [PATCH 1/6] Use canvas.toBlob() instead of fetch() to create a blob from image src --- .../src/imageupload/imageuploadediting.js | 4 +- .../ckeditor5-image/src/imageupload/utils.js | 63 ++++++--- .../tests/imageupload/imageuploadediting.js | 125 ++++++++++++------ .../tests/manual/tickets/7957/1.html | 14 ++ .../tests/manual/tickets/7957/1.js | 42 ++++++ .../tests/manual/tickets/7957/1.md | 7 + 6 files changed, 189 insertions(+), 66 deletions(-) create mode 100644 packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.html create mode 100644 packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.js create mode 100644 packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.md diff --git a/packages/ckeditor5-image/src/imageupload/imageuploadediting.js b/packages/ckeditor5-image/src/imageupload/imageuploadediting.js index 7dcaa143861..8bb894730c3 100644 --- a/packages/ckeditor5-image/src/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/src/imageupload/imageuploadediting.js @@ -15,7 +15,7 @@ import UpcastWriter from '@ckeditor/ckeditor5-engine/src/view/upcastwriter'; import env from '@ckeditor/ckeditor5-utils/src/env'; import ImageUploadCommand from '../../src/imageupload/imageuploadcommand'; -import { fetchLocalImage, isLocalImage } from '../../src/imageupload/utils'; +import { createImageFile, isLocalImage } from '../../src/imageupload/utils'; import { createImageTypeRegExp } from './utils'; import { getViewImgFromWidget } from '../image/utils'; @@ -123,7 +123,7 @@ export default class ImageUploadEditing extends Plugin { this.listenTo( editor.plugins.get( Clipboard ), 'inputTransformation', ( evt, data ) => { const fetchableImages = Array.from( editor.editing.view.createRangeIn( data.content ) ) .filter( value => isLocalImage( value.item ) && !value.item.getAttribute( 'uploadProcessed' ) ) - .map( value => { return { promise: fetchLocalImage( value.item ), imageElement: value.item }; } ); + .map( value => { return { promise: createImageFile( value.item ), imageElement: value.item }; } ); if ( !fetchableImages.length ) { return; diff --git a/packages/ckeditor5-image/src/imageupload/utils.js b/packages/ckeditor5-image/src/imageupload/utils.js index 22cc43861a8..6b56cde6dea 100644 --- a/packages/ckeditor5-image/src/imageupload/utils.js +++ b/packages/ckeditor5-image/src/imageupload/utils.js @@ -7,7 +7,9 @@ * @module image/imageupload/utils */ -/* global fetch, File */ +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; + +/* global File */ /** * Creates a regular expression used to test for image files. @@ -27,28 +29,22 @@ export function createImageTypeRegExp( types ) { } /** - * Creates a promise that fetches the image local source (Base64 or blob) and resolves with a `File` object. + * Creates a promise that converts the image local source (Base64 or blob) to a blob and resolves with a `File` object. * - * @param {module:engine/view/element~Element} image Image whose source to fetch. - * @returns {Promise.} A promise which resolves when an image source is fetched and converted to a `File` instance. + * @param {module:engine/view/element~Element} image Image whose source to convert. + * @returns {Promise.} A promise which resolves when an image source is converted to a `File` instance. * It resolves with a `File` object. If there were any errors during file processing, the promise will be rejected. */ -export function fetchLocalImage( image ) { - return new Promise( ( resolve, reject ) => { - const imageSrc = image.getAttribute( 'src' ); - - // Fetch works asynchronously and so does not block browser UI when processing data. - fetch( imageSrc ) - .then( resource => resource.blob() ) - .then( blob => { - const mimeType = getImageMimeType( blob, imageSrc ); - const ext = mimeType.replace( 'image/', '' ); - const filename = `image.${ ext }`; - const file = new File( [ blob ], filename, { type: mimeType } ); - - resolve( file ); - } ) - .catch( reject ); +export function createImageFile( image ) { + const imageSrc = image.getAttribute( 'src' ); + + // Conversion to blob works asynchronously, so it does not block browser UI when processing data. + return getBlobFromImage( imageSrc ).then( blob => { + const mimeType = getImageMimeType( blob, imageSrc ); + const ext = mimeType.replace( 'image/', '' ); + const filename = `image.${ ext }`; + + return new File( [ blob ], filename, { type: mimeType } ); } ); } @@ -67,6 +63,33 @@ export function isLocalImage( node ) { node.getAttribute( 'src' ).match( /^blob:/g ); } +// Creates a promise that resolves with a `Blob` object converted from the image source (Base64 or blob). +// +// @param {String} imageSrc Image `src` attribute value. +// @returns {Promise.} +function getBlobFromImage( imageSrc ) { + return new Promise( ( resolve, reject ) => { + const image = global.document.createElement( 'img' ); + + image.addEventListener( 'load', () => { + const canvas = global.document.createElement( 'canvas' ); + + canvas.width = image.width; + canvas.height = image.height; + + const ctx = canvas.getContext( '2d' ); + + ctx.drawImage( image, 0, 0 ); + + canvas.toBlob( blob => blob ? resolve( blob ) : reject() ); + } ); + + image.addEventListener( 'error', reject ); + + image.src = imageSrc; + } ); +} + // Extracts an image type based on its blob representation or its source. // // @param {String} src Image `src` attribute value. diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js index 18c18f8ee87..cf437284854 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals window, setTimeout, atob, URL, Blob, console */ +/* globals document, window, setTimeout, atob, URL, Blob, HTMLCanvasElement, console */ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; @@ -28,6 +28,7 @@ import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification'; describe( 'ImageUploadEditing', () => { // eslint-disable-next-line max-len const base64Sample = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII='; + const base64InvalidSample = 'data:image/png;base64,INVALID-DATA'; let adapterMocks = []; let editor, model, view, doc, fileRepository, viewDocument, nativeReaderMock, loader; @@ -671,7 +672,7 @@ describe( 'ImageUploadEditing', () => { expectModel( done, getModelData( model ), expected ); } ); - it( 'should not upload and remove image if fetch failed', done => { + it( 'should not upload and remove image if conversion to a blob failed', done => { const notification = editor.plugins.get( Notification ); // Prevent popping up alert window. @@ -681,17 +682,12 @@ describe( 'ImageUploadEditing', () => { setModelData( model, '[]foo' ); - const clipboardHtml = ``; + const clipboardHtml = ``; const dataTransfer = mockDataTransfer( clipboardHtml ); const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); - // Stub `fetch` so it can be rejected. - sinon.stub( window, 'fetch' ).callsFake( () => { - return new Promise( ( res, rej ) => rej( 'could not fetch' ) ); - } ); - let content = null; editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', ( evt, data ) => { content = data.content; @@ -709,7 +705,7 @@ describe( 'ImageUploadEditing', () => { ); } ); - it( 'should upload only images which were successfully fetched and remove failed ones', done => { + it( 'should upload only images which were successfully converted to a blob and remove failed ones', done => { const notification = editor.plugins.get( Notification ); // Prevent popping up alert window. @@ -729,25 +725,15 @@ describe( 'ImageUploadEditing', () => { setModelData( model, '[]foo' ); + // The first 2 images are valid ones, and the 3rd one fails. const clipboardHtml = `

bar

` + - ``; + `` + + ``; const dataTransfer = mockDataTransfer( clipboardHtml ); const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); - // Stub `fetch` in a way that 2 first calls are successful and 3rd fails. - let counter = 0; - const fetch = window.fetch; - sinon.stub( window, 'fetch' ).callsFake( src => { - counter++; - if ( counter < 3 ) { - return fetch( src ); - } else { - return Promise.reject(); - } - } ); - let content = null; editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', ( evt, data ) => { content = data.content; @@ -848,15 +834,6 @@ describe( 'ImageUploadEditing', () => { const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); - // Stub `fetch` to return custom blob without type. - sinon.stub( window, 'fetch' ).callsFake( () => { - return new Promise( res => res( { - blob() { - return new Promise( res => res( new Blob( [ 'foo', 'bar' ] ) ) ); - } - } ) ); - } ); - viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); tryExpect( done, () => { @@ -873,15 +850,6 @@ describe( 'ImageUploadEditing', () => { const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); - // Stub `fetch` to return custom blob without type. - sinon.stub( window, 'fetch' ).callsFake( () => { - return new Promise( res => res( { - blob() { - return new Promise( res => res( new Blob( [ 'foo', 'bar' ] ) ) ); - } - } ) ); - } ); - viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); tryExpect( done, () => { @@ -907,8 +875,8 @@ describe( 'ImageUploadEditing', () => { const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); - // Stub `fetch` in a way that it always fails. - sinon.stub( window, 'fetch' ).callsFake( () => Promise.reject() ); + // Stub `HTMLCanvasElement#toBlob` to return invalid blob, so image conversion always fails. + sinon.stub( HTMLCanvasElement.prototype, 'toBlob' ).callsFake( fn => fn( null ) ); viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); @@ -923,13 +891,82 @@ describe( 'ImageUploadEditing', () => { } ); } ); + describe( 'with Content Security Policy rules', () => { + let metaElement; + let previousMetaContent; + + // Set the CSP rules before the first test in this block has been executed. + before( () => { + metaElement = document.querySelector( '[http-equiv=Content-Security-Policy]' ); + + if ( metaElement ) { + previousMetaContent = metaElement.getAttribute( 'content' ); + } else { + metaElement = document.createElement( 'meta' ); + metaElement.setAttribute( 'http-equiv', 'Content-Security-Policy' ); + + document.head.appendChild( metaElement ); + } + + metaElement.setAttribute( 'content', '' + + 'default-src \'none\'; ' + + 'connect-src \'self\'; ' + + 'script-src \'self\'; ' + + 'img-src * data: blob:;' + + 'style-src \'self\' \'unsafe-inline\'; ' + + 'frame-src *' + ); + } ); + + // Remove or restore the previous CSP rules after the last test in this block has been executed. + after( () => { + if ( previousMetaContent ) { + metaElement.setAttribute( 'content', previousMetaContent ); + } else { + document.head.removeChild( metaElement ); + } + } ); + + // See https://github.com/ckeditor/ckeditor5/issues/7957. + it( 'should upload image if strict CSP rules are defined', done => { + const spy = sinon.spy(); + const notification = editor.plugins.get( Notification ); + + notification.on( 'show:warning', evt => { + spy(); + evt.stop(); + }, { priority: 'high' } ); + + setModelData( model, '[]foo' ); + + const clipboardHtml = `

bar

`; + const dataTransfer = mockDataTransfer( clipboardHtml ); + + const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); + const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + + viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); + + adapterMocks[ 0 ].loader.file.then( () => { + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + } ); + } ).catch( () => { + setTimeout( () => { + expect.fail( 'Promise should be resolved.' ); + } ); + } ); + } ); + } ); + // Helper for validating clipboard and model data as a result of a paste operation. This function checks both clipboard // data and model data synchronously (`expectedClipboardData`, `expectedModel`) and then the model data after `loader.file` - // promise is resolved (so model state after successful/failed file fetch attempt). + // promise is resolved (so model state after successful/failed file conversion attempt). // // @param {String} expectedClipboardData Expected clipboard data on `inputTransformation` event. // @param {String} expectedModel Expected model data on `inputTransformation` event. - // @param {String} expectedModelOnFile Expected model data after all `file.loader` promises are fetched. + // @param {String} expectedModelOnFile Expected model data after all `file.loader` promises are fulfilled. // @param {DocumentFragment} content Content processed in inputTransformation // @param {Function} doneFn Callback function to be called when all assertions are done or error occures. // @param {Boolean} [onSuccess=true] If `expectedModelOnFile` data should be validated diff --git a/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.html b/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.html new file mode 100644 index 00000000000..f605f98b253 --- /dev/null +++ b/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.html @@ -0,0 +1,14 @@ + + + + +
+

Paste here:

+
diff --git a/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.js b/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.js new file mode 100644 index 00000000000..f74203075cc --- /dev/null +++ b/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.js @@ -0,0 +1,42 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; + +import Strikethrough from '@ckeditor/ckeditor5-basic-styles/src/strikethrough'; +import Underline from '@ckeditor/ckeditor5-basic-styles/src/underline'; +import Table from '@ckeditor/ckeditor5-table/src/table'; +import TableToolbar from '@ckeditor/ckeditor5-table/src/tabletoolbar'; +import EasyImage from '@ckeditor/ckeditor5-easy-image/src/easyimage'; +import FontColor from '@ckeditor/ckeditor5-font/src/fontcolor'; +import FontBackgroundColor from '@ckeditor/ckeditor5-font/src/fontbackgroundcolor'; +import PageBreak from '@ckeditor/ckeditor5-page-break/src/pagebreak'; +import TableProperties from '@ckeditor/ckeditor5-table/src/tableproperties'; +import TableCellProperties from '@ckeditor/ckeditor5-table/src/tablecellproperties'; + +import PasteFromOffice from '../../../../src/pastefromoffice'; + +import { CS_CONFIG } from '@ckeditor/ckeditor5-cloud-services/tests/_utils/cloud-services-config'; + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ ArticlePluginSet, Strikethrough, Underline, Table, TableToolbar, PageBreak, + TableProperties, TableCellProperties, EasyImage, PasteFromOffice, FontColor, FontBackgroundColor ], + toolbar: [ 'heading', '|', 'bold', 'italic', 'strikethrough', 'underline', 'link', + 'bulletedList', 'numberedList', 'blockQuote', 'insertTable', 'pageBreak', 'undo', 'redo' ], + table: { + contentToolbar: [ 'tableColumn', 'tableRow', 'mergeTableCells', 'tableProperties', 'tableCellProperties' ] + }, + cloudServices: CS_CONFIG + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.md b/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.md new file mode 100644 index 00000000000..18c5a1f3ec2 --- /dev/null +++ b/packages/ckeditor5-paste-from-office/tests/manual/tickets/7957/1.md @@ -0,0 +1,7 @@ +## Paste from Office + +Test for Paste from Word, when strict CSP rules are configured. + +Check: + +1. Copy & paste some content from Word including at least one image. From a0af560396859f293883ff7e328a142f3834f2ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Fri, 18 Dec 2020 15:02:26 +0100 Subject: [PATCH 2/6] Fixed code coverage --- .../ckeditor5-image/src/imageupload/utils.js | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/ckeditor5-image/src/imageupload/utils.js b/packages/ckeditor5-image/src/imageupload/utils.js index 6b56cde6dea..92997eff111 100644 --- a/packages/ckeditor5-image/src/imageupload/utils.js +++ b/packages/ckeditor5-image/src/imageupload/utils.js @@ -40,7 +40,7 @@ export function createImageFile( image ) { // Conversion to blob works asynchronously, so it does not block browser UI when processing data. return getBlobFromImage( imageSrc ).then( blob => { - const mimeType = getImageMimeType( blob, imageSrc ); + const mimeType = getImageMimeType( imageSrc ); const ext = mimeType.replace( 'image/', '' ); const filename = `image.${ ext }`; @@ -81,7 +81,11 @@ function getBlobFromImage( imageSrc ) { ctx.drawImage( image, 0, 0 ); - canvas.toBlob( blob => blob ? resolve( blob ) : reject() ); + canvas.toBlob( + blob => blob ? resolve( blob ) : reject(), + getImageMimeType( imageSrc ), + 1 + ); } ); image.addEventListener( 'error', reject ); @@ -90,18 +94,13 @@ function getBlobFromImage( imageSrc ) { } ); } -// Extracts an image type based on its blob representation or its source. +// Extracts an image type based on its source. // // @param {String} src Image `src` attribute value. -// @param {Blob} blob Image blob representation. // @returns {String} -function getImageMimeType( blob, src ) { - if ( blob.type ) { - return blob.type; - } else if ( src.match( /data:(image\/\w+);base64/ ) ) { - return src.match( /data:(image\/\w+);base64/ )[ 1 ].toLowerCase(); - } else { - // Fallback to 'jpeg' as common extension. - return 'image/jpeg'; - } +function getImageMimeType( src ) { + const match = src.match( /data:(image\/\w+);base64/ ); + + // Fallback to 'jpeg' as common extension. + return match ? match[ 1 ].toLowerCase() : 'image/jpeg'; } From 8c6c7a15967ef284ba8e8e495277596c4b9536af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Tue, 22 Dec 2020 08:05:37 +0100 Subject: [PATCH 3/6] Call getImageMimeType() once --- packages/ckeditor5-image/src/imageupload/utils.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/ckeditor5-image/src/imageupload/utils.js b/packages/ckeditor5-image/src/imageupload/utils.js index 92997eff111..968b59de1d5 100644 --- a/packages/ckeditor5-image/src/imageupload/utils.js +++ b/packages/ckeditor5-image/src/imageupload/utils.js @@ -37,10 +37,10 @@ export function createImageTypeRegExp( types ) { */ export function createImageFile( image ) { const imageSrc = image.getAttribute( 'src' ); + const mimeType = getImageMimeType( imageSrc ); // Conversion to blob works asynchronously, so it does not block browser UI when processing data. - return getBlobFromImage( imageSrc ).then( blob => { - const mimeType = getImageMimeType( imageSrc ); + return getBlobFromImage( imageSrc, mimeType ).then( blob => { const ext = mimeType.replace( 'image/', '' ); const filename = `image.${ ext }`; @@ -66,8 +66,9 @@ export function isLocalImage( node ) { // Creates a promise that resolves with a `Blob` object converted from the image source (Base64 or blob). // // @param {String} imageSrc Image `src` attribute value. +// @param {String} mimeType Image type based on its source. // @returns {Promise.} -function getBlobFromImage( imageSrc ) { +function getBlobFromImage( imageSrc, mimeType ) { return new Promise( ( resolve, reject ) => { const image = global.document.createElement( 'img' ); @@ -83,7 +84,7 @@ function getBlobFromImage( imageSrc ) { canvas.toBlob( blob => blob ? resolve( blob ) : reject(), - getImageMimeType( imageSrc ), + mimeType, 1 ); } ); From 9079fcd1ab727d1640012b4d6f3df6d2d64c50a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Mon, 28 Dec 2020 14:13:57 +0100 Subject: [PATCH 4/6] Use canvas conversion as a fallback only if fetch() has failed --- .../src/imageupload/imageuploadediting.js | 4 +- .../ckeditor5-image/src/imageupload/utils.js | 92 ++++++++++++------- .../tests/imageupload/imageuploadediting.js | 90 +++++++++++++++--- 3 files changed, 138 insertions(+), 48 deletions(-) diff --git a/packages/ckeditor5-image/src/imageupload/imageuploadediting.js b/packages/ckeditor5-image/src/imageupload/imageuploadediting.js index 8bb894730c3..7dcaa143861 100644 --- a/packages/ckeditor5-image/src/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/src/imageupload/imageuploadediting.js @@ -15,7 +15,7 @@ import UpcastWriter from '@ckeditor/ckeditor5-engine/src/view/upcastwriter'; import env from '@ckeditor/ckeditor5-utils/src/env'; import ImageUploadCommand from '../../src/imageupload/imageuploadcommand'; -import { createImageFile, isLocalImage } from '../../src/imageupload/utils'; +import { fetchLocalImage, isLocalImage } from '../../src/imageupload/utils'; import { createImageTypeRegExp } from './utils'; import { getViewImgFromWidget } from '../image/utils'; @@ -123,7 +123,7 @@ export default class ImageUploadEditing extends Plugin { this.listenTo( editor.plugins.get( Clipboard ), 'inputTransformation', ( evt, data ) => { const fetchableImages = Array.from( editor.editing.view.createRangeIn( data.content ) ) .filter( value => isLocalImage( value.item ) && !value.item.getAttribute( 'uploadProcessed' ) ) - .map( value => { return { promise: createImageFile( value.item ), imageElement: value.item }; } ); + .map( value => { return { promise: fetchLocalImage( value.item ), imageElement: value.item }; } ); if ( !fetchableImages.length ) { return; diff --git a/packages/ckeditor5-image/src/imageupload/utils.js b/packages/ckeditor5-image/src/imageupload/utils.js index 968b59de1d5..1a31be50791 100644 --- a/packages/ckeditor5-image/src/imageupload/utils.js +++ b/packages/ckeditor5-image/src/imageupload/utils.js @@ -9,7 +9,7 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; -/* global File */ +/* global fetch File */ /** * Creates a regular expression used to test for image files. @@ -29,22 +29,35 @@ export function createImageTypeRegExp( types ) { } /** - * Creates a promise that converts the image local source (Base64 or blob) to a blob and resolves with a `File` object. + * Creates a promise that fetches the image local source (Base64 or blob) and resolves with a `File` object. * - * @param {module:engine/view/element~Element} image Image whose source to convert. - * @returns {Promise.} A promise which resolves when an image source is converted to a `File` instance. + * @param {module:engine/view/element~Element} image Image whose source to fetch. + * @returns {Promise.} A promise which resolves when an image source is fetched and converted to a `File` instance. * It resolves with a `File` object. If there were any errors during file processing, the promise will be rejected. */ -export function createImageFile( image ) { - const imageSrc = image.getAttribute( 'src' ); - const mimeType = getImageMimeType( imageSrc ); - - // Conversion to blob works asynchronously, so it does not block browser UI when processing data. - return getBlobFromImage( imageSrc, mimeType ).then( blob => { - const ext = mimeType.replace( 'image/', '' ); - const filename = `image.${ ext }`; - - return new File( [ blob ], filename, { type: mimeType } ); +export function fetchLocalImage( image ) { + return new Promise( ( resolve, reject ) => { + const imageSrc = image.getAttribute( 'src' ); + + // Fetch works asynchronously and so does not block browser UI when processing data. + fetch( imageSrc ) + .then( resource => resource.blob() ) + .then( blob => { + const mimeType = getImageMimeType( blob, imageSrc ); + const ext = mimeType.replace( 'image/', '' ); + const filename = `image.${ ext }`; + const file = new File( [ blob ], filename, { type: mimeType } ); + + resolve( file ); + } ) + .catch( err => { + // Fetch fails only, if it can't make a request due to a network failure or if anything prevented the request + // from completing, i.e. the Content Security Policy rules. It is not possible to detect the exact cause of failure, + // so we are just trying the fallback solution, if general TypeError is thrown. + return err && err.name === 'TypeError' ? + convertLocalImageOnCanvas( imageSrc ).then( resolve ).catch( reject ) : + reject( err ); + } ); } ); } @@ -63,12 +76,42 @@ export function isLocalImage( node ) { node.getAttribute( 'src' ).match( /^blob:/g ); } +// Extracts an image type based on its blob representation or its source. +// +// @param {String} src Image `src` attribute value. +// @param {Blob} blob Image blob representation. +// @returns {String} +function getImageMimeType( blob, src ) { + if ( blob.type ) { + return blob.type; + } else if ( src.match( /data:(image\/\w+);base64/ ) ) { + return src.match( /data:(image\/\w+);base64/ )[ 1 ].toLowerCase(); + } else { + // Fallback to 'jpeg' as common extension. + return 'image/jpeg'; + } +} + +// Creates a promise that converts the image local source (Base64 or blob) to a blob and resolves with a `File` object. +// +// @param {String} imageSrc Image `src` attribute value. +// @returns {Promise.} A promise which resolves when an image source is converted to a `File` instance. +// It resolves with a `File` object. If there were any errors during file processing, the promise will be rejected. +function convertLocalImageOnCanvas( imageSrc ) { + return getBlobFromCanvas( imageSrc ).then( blob => { + const mimeType = getImageMimeType( blob, imageSrc ); + const ext = mimeType.replace( 'image/', '' ); + const filename = `image.${ ext }`; + + return new File( [ blob ], filename, { type: mimeType } ); + } ); +} + // Creates a promise that resolves with a `Blob` object converted from the image source (Base64 or blob). // // @param {String} imageSrc Image `src` attribute value. -// @param {String} mimeType Image type based on its source. // @returns {Promise.} -function getBlobFromImage( imageSrc, mimeType ) { +function getBlobFromCanvas( imageSrc ) { return new Promise( ( resolve, reject ) => { const image = global.document.createElement( 'img' ); @@ -82,11 +125,7 @@ function getBlobFromImage( imageSrc, mimeType ) { ctx.drawImage( image, 0, 0 ); - canvas.toBlob( - blob => blob ? resolve( blob ) : reject(), - mimeType, - 1 - ); + canvas.toBlob( blob => blob ? resolve( blob ) : reject() ); } ); image.addEventListener( 'error', reject ); @@ -94,14 +133,3 @@ function getBlobFromImage( imageSrc, mimeType ) { image.src = imageSrc; } ); } - -// Extracts an image type based on its source. -// -// @param {String} src Image `src` attribute value. -// @returns {String} -function getImageMimeType( src ) { - const match = src.match( /data:(image\/\w+);base64/ ); - - // Fallback to 'jpeg' as common extension. - return match ? match[ 1 ].toLowerCase() : 'image/jpeg'; -} diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js index cf437284854..5fe144efb66 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js @@ -28,7 +28,6 @@ import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification'; describe( 'ImageUploadEditing', () => { // eslint-disable-next-line max-len const base64Sample = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII='; - const base64InvalidSample = 'data:image/png;base64,INVALID-DATA'; let adapterMocks = []; let editor, model, view, doc, fileRepository, viewDocument, nativeReaderMock, loader; @@ -672,7 +671,7 @@ describe( 'ImageUploadEditing', () => { expectModel( done, getModelData( model ), expected ); } ); - it( 'should not upload and remove image if conversion to a blob failed', done => { + it( 'should not upload and remove image if fetch failed', done => { const notification = editor.plugins.get( Notification ); // Prevent popping up alert window. @@ -682,12 +681,17 @@ describe( 'ImageUploadEditing', () => { setModelData( model, '[]foo' ); - const clipboardHtml = ``; + const clipboardHtml = ``; const dataTransfer = mockDataTransfer( clipboardHtml ); const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + // Stub `fetch` so it can be rejected. + sinon.stub( window, 'fetch' ).callsFake( () => { + return new Promise( ( res, rej ) => rej( 'could not fetch' ) ); + } ); + let content = null; editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', ( evt, data ) => { content = data.content; @@ -705,7 +709,7 @@ describe( 'ImageUploadEditing', () => { ); } ); - it( 'should upload only images which were successfully converted to a blob and remove failed ones', done => { + it( 'should upload only images which were successfully fetched and remove failed ones', done => { const notification = editor.plugins.get( Notification ); // Prevent popping up alert window. @@ -725,15 +729,25 @@ describe( 'ImageUploadEditing', () => { setModelData( model, '[]foo' ); - // The first 2 images are valid ones, and the 3rd one fails. const clipboardHtml = `

bar

` + - `` + - ``; + ``; const dataTransfer = mockDataTransfer( clipboardHtml ); const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + // Stub `fetch` in a way that 2 first calls are successful and 3rd fails. + let counter = 0; + const fetch = window.fetch; + sinon.stub( window, 'fetch' ).callsFake( src => { + counter++; + if ( counter < 3 ) { + return fetch( src ); + } else { + return Promise.reject(); + } + } ); + let content = null; editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', ( evt, data ) => { content = data.content; @@ -834,6 +848,15 @@ describe( 'ImageUploadEditing', () => { const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + // Stub `fetch` to return custom blob without type. + sinon.stub( window, 'fetch' ).callsFake( () => { + return new Promise( res => res( { + blob() { + return new Promise( res => res( new Blob( [ 'foo', 'bar' ] ) ) ); + } + } ) ); + } ); + viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); tryExpect( done, () => { @@ -850,6 +873,15 @@ describe( 'ImageUploadEditing', () => { const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + // Stub `fetch` to return custom blob without type. + sinon.stub( window, 'fetch' ).callsFake( () => { + return new Promise( res => res( { + blob() { + return new Promise( res => res( new Blob( [ 'foo', 'bar' ] ) ) ); + } + } ) ); + } ); + viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); tryExpect( done, () => { @@ -875,8 +907,8 @@ describe( 'ImageUploadEditing', () => { const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); - // Stub `HTMLCanvasElement#toBlob` to return invalid blob, so image conversion always fails. - sinon.stub( HTMLCanvasElement.prototype, 'toBlob' ).callsFake( fn => fn( null ) ); + // Stub `fetch` in a way that it always fails. + sinon.stub( window, 'fetch' ).callsFake( () => Promise.reject() ); viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); @@ -891,11 +923,12 @@ describe( 'ImageUploadEditing', () => { } ); } ); - describe( 'with Content Security Policy rules', () => { + describe( 'fallback image conversion on canvas', () => { let metaElement; let previousMetaContent; - // Set the CSP rules before the first test in this block has been executed. + // Set strict Content Security Policy (CSP) rules before the first test in this block has been executed. + // The CSP rules cause that fetch() fails and it triggers the fallback procedure. before( () => { metaElement = document.querySelector( '[http-equiv=Content-Security-Policy]' ); @@ -928,7 +961,7 @@ describe( 'ImageUploadEditing', () => { } ); // See https://github.com/ckeditor/ckeditor5/issues/7957. - it( 'should upload image if strict CSP rules are defined', done => { + it( 'should upload image using canvas conversion', done => { const spy = sinon.spy(); const notification = editor.plugins.get( Notification ); @@ -958,15 +991,44 @@ describe( 'ImageUploadEditing', () => { } ); } ); } ); + + it( 'should not upload and remove image if canvas convertion failed', done => { + setModelData( model, '[]foo' ); + + const clipboardHtml = ``; + const dataTransfer = mockDataTransfer( clipboardHtml ); + + const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); + const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + + // Stub `HTMLCanvasElement#toBlob` to return invalid blob, so image conversion always fails. + sinon.stub( HTMLCanvasElement.prototype, 'toBlob' ).callsFake( fn => fn( null ) ); + + let content = null; + editor.plugins.get( 'Clipboard' ).on( 'inputTransformation', ( evt, data ) => { + content = data.content; + } ); + + viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); + + expectData( + '', + '[]foo', + '[]foo', + content, + done, + false + ); + } ); } ); // Helper for validating clipboard and model data as a result of a paste operation. This function checks both clipboard // data and model data synchronously (`expectedClipboardData`, `expectedModel`) and then the model data after `loader.file` - // promise is resolved (so model state after successful/failed file conversion attempt). + // promise is resolved (so model state after successful/failed file fetch attempt). // // @param {String} expectedClipboardData Expected clipboard data on `inputTransformation` event. // @param {String} expectedModel Expected model data on `inputTransformation` event. - // @param {String} expectedModelOnFile Expected model data after all `file.loader` promises are fulfilled. + // @param {String} expectedModelOnFile Expected model data after all `file.loader` promises are fetched. // @param {DocumentFragment} content Content processed in inputTransformation // @param {Function} doneFn Callback function to be called when all assertions are done or error occures. // @param {Boolean} [onSuccess=true] If `expectedModelOnFile` data should be validated From ac238166a3c10499cd4692b25305781e16a670f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Mon, 28 Dec 2020 14:20:58 +0100 Subject: [PATCH 5/6] Cosmetic clean-up --- packages/ckeditor5-image/src/imageupload/utils.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-image/src/imageupload/utils.js b/packages/ckeditor5-image/src/imageupload/utils.js index 1a31be50791..be8d1bd5e49 100644 --- a/packages/ckeditor5-image/src/imageupload/utils.js +++ b/packages/ckeditor5-image/src/imageupload/utils.js @@ -7,9 +7,9 @@ * @module image/imageupload/utils */ -import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +/* global fetch, File */ -/* global fetch File */ +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; /** * Creates a regular expression used to test for image files. @@ -92,7 +92,8 @@ function getImageMimeType( blob, src ) { } } -// Creates a promise that converts the image local source (Base64 or blob) to a blob and resolves with a `File` object. +// Creates a promise that converts the image local source (Base64 or blob) to a blob using canvas and resolves +// with a `File` object. // // @param {String} imageSrc Image `src` attribute value. // @returns {Promise.} A promise which resolves when an image source is converted to a `File` instance. From 402e793b24dc4246c7205aa8b58626ec21021deb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Smyrek?= Date: Tue, 12 Jan 2021 10:41:09 +0100 Subject: [PATCH 6/6] Prevent showing an alert() with event object --- .../ckeditor5-image/src/imageupload/utils.js | 2 +- .../tests/imageupload/imageuploadediting.js | 31 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-image/src/imageupload/utils.js b/packages/ckeditor5-image/src/imageupload/utils.js index be8d1bd5e49..9230bb0992b 100644 --- a/packages/ckeditor5-image/src/imageupload/utils.js +++ b/packages/ckeditor5-image/src/imageupload/utils.js @@ -129,7 +129,7 @@ function getBlobFromCanvas( imageSrc ) { canvas.toBlob( blob => blob ? resolve( blob ) : reject() ); } ); - image.addEventListener( 'error', reject ); + image.addEventListener( 'error', () => reject() ); image.src = imageSrc; } ); diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js index 5fe144efb66..74da5ab5ba7 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadediting.js @@ -992,7 +992,7 @@ describe( 'ImageUploadEditing', () => { } ); } ); - it( 'should not upload and remove image if canvas convertion failed', done => { + it( 'should not upload and remove image if canvas conversion failed', done => { setModelData( model, '[]foo' ); const clipboardHtml = ``; @@ -1020,6 +1020,35 @@ describe( 'ImageUploadEditing', () => { false ); } ); + + it( 'should not show notification when image could not be loaded', done => { + const spy = sinon.spy(); + const notification = editor.plugins.get( Notification ); + + notification.on( 'show:warning', evt => { + spy(); + evt.stop(); + }, { priority: 'high' } ); + + setModelData( model, '[]foo' ); + + const clipboardHtml = ''; + const dataTransfer = mockDataTransfer( clipboardHtml ); + + const targetRange = model.createRange( model.createPositionAt( doc.getRoot(), 1 ), model.createPositionAt( doc.getRoot(), 1 ) ); + const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); + + viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); + + adapterMocks[ 0 ].loader.file.then( () => { + expect.fail( 'Promise should be rejected.' ); + } ).catch( () => { + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + } ); + } ); + } ); } ); // Helper for validating clipboard and model data as a result of a paste operation. This function checks both clipboard