diff --git a/ghost/core/core/server/adapters/storage/LocalStorageBase.js b/ghost/core/core/server/adapters/storage/LocalStorageBase.js index 915a30a81fd..fa6609aa898 100644 --- a/ghost/core/core/server/adapters/storage/LocalStorageBase.js +++ b/ghost/core/core/server/adapters/storage/LocalStorageBase.js @@ -43,23 +43,30 @@ class LocalStorageBase extends StorageBase { * Saves the file to storage (the file system) * - returns a promise which ultimately returns the full url to the uploaded file * - * @param {StorageBase.Image} file + * @param {Object} file + * @param {String} [file.path] -- The original path of the file + * @param {String} [file.name] -- The original name of the file + * @param {Boolean} [file.keepOriginalName] -- If true, skip generating a new filename * @param {String} targetDir * @returns {Promise} */ async save(file, targetDir) { - let targetFilename; - - // NOTE: the base implementation of `getTargetDir` returns the format this.storagePath/YYYY/MM - targetDir = targetDir || this.getTargetDir(this.storagePath); - - const filename = await this.getUniqueFileName(file, targetDir); - - targetFilename = filename; - await fs.mkdirs(targetDir); + // The base implementation of `getTargetDir` returns the format this.storagePath/YYYY/MM, e.g. /content/images/2025/01 + const directory = targetDir || this.getTargetDir(this.storagePath); + const originalFilePath = file.path; + + // If the `keepOriginalName` flag is set, don't generate a new filename + // Otherwise, generate a unique secure filename, composed of a 16-character random hash and truncated to be under 255 bytes, e.g. image-a1b2c3d4e5f6g789.png + let targetFilePath; + if (file.keepOriginalName) { + targetFilePath = path.join(directory, file.name); + } else { + targetFilePath = this.getUniqueSecureFilePath(file, directory); + } try { - await fs.copy(file.path, targetFilename); + await fs.mkdirs(directory); + await fs.copy(originalFilePath, targetFilePath); } catch (err) { if (err.code === 'ENAMETOOLONG') { throw new errors.BadRequestError({err}); @@ -74,7 +81,7 @@ class LocalStorageBase extends StorageBase { urlUtils.urlJoin('/', urlUtils.getSubdir(), this.staticFileURLPrefix, - path.relative(this.storagePath, targetFilename)) + path.relative(this.storagePath, targetFilePath)) ).replace(new RegExp(`\\${path.sep}`, 'g'), '/'); return fullUrl; diff --git a/ghost/core/core/server/api/endpoints/images.js b/ghost/core/core/server/api/endpoints/images.js index b95dedc0abd..ce89d34b14e 100644 --- a/ghost/core/core/server/api/endpoints/images.js +++ b/ghost/core/core/server/api/endpoints/images.js @@ -71,7 +71,8 @@ const controller = { await store.save({ ...frame.file, path: originalPath, - name: imageTransform.generateOriginalImageName(processedImageName) + name: imageTransform.generateOriginalImageName(processedImageName), + keepOriginalName: true // Don't generate a new filename on save }, processedImageDir); return processedImageUrl; diff --git a/ghost/core/core/server/services/themes/storage.js b/ghost/core/core/server/services/themes/storage.js index f087e37e133..7e7f45234df 100644 --- a/ghost/core/core/server/services/themes/storage.js +++ b/ghost/core/core/server/services/themes/storage.js @@ -46,7 +46,7 @@ module.exports = { }); }, setFromZip: async (zip) => { - const themeName = getStorage().getSanitizedFileName(zip.name.split('.zip')[0]); + const themeName = getStorage().sanitizeFileName(zip.name.split('.zip')[0]); const backupName = `${themeName}_${ObjectID()}`; // check if zip name matches one of the default themes @@ -63,6 +63,7 @@ module.exports = { try { checkedTheme = await validate.checkSafe(themeName, zip, true); const themeExists = await getStorage().exists(themeName); + // CASE: move the existing theme to a backup folder if (themeExists) { debug('setFromZip Theme exists already'); @@ -73,7 +74,8 @@ module.exports = { // CASE: store extracted theme await getStorage().save({ name: themeName, - path: checkedTheme.path + path: checkedTheme.path, + keepOriginalName: true }); // CASE: loads the theme from the fs & sets the theme on the themeList diff --git a/ghost/core/package.json b/ghost/core/package.json index ce75556c25e..a18c8194912 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -189,7 +189,7 @@ "express-queue": "0.0.13", "express-session": "1.18.1", "fs-extra": "11.2.0", - "ghost-storage-base": "1.0.0", + "ghost-storage-base": "1.1.1", "glob": "8.1.0", "got": "11.8.6", "gscan": "4.46.0", diff --git a/ghost/core/test/e2e-api/admin/files.test.js b/ghost/core/test/e2e-api/admin/files.test.js index 300e0294a9c..a5f0f11c57e 100644 --- a/ghost/core/test/e2e-api/admin/files.test.js +++ b/ghost/core/test/e2e-api/admin/files.test.js @@ -29,7 +29,7 @@ describe('Files API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/images/loadingcat_square.gif')) .expect(201); - res.body.files[0].url.should.match(new RegExp(`${config.get('url')}/content/files/\\d+/\\d+/loadingcat_square.gif`)); + res.body.files[0].url.should.match(new RegExp(`${config.get('url')}/content/files/\\d+/\\d+/loadingcat_square-\\w{16}\\.gif`)); res.body.files[0].ref.should.equal('934203942'); files.push(res.body.files[0].url.replace(config.get('url'), '')); diff --git a/ghost/core/test/e2e-api/admin/images.test.js b/ghost/core/test/e2e-api/admin/images.test.js index db3679a3501..e3dbbfde66b 100644 --- a/ghost/core/test/e2e-api/admin/images.test.js +++ b/ghost/core/test/e2e-api/admin/images.test.js @@ -13,6 +13,7 @@ const {anyErrorId} = matchers; const {imageSize} = require('../../../core/server/lib/image'); const configUtils = require('../../utils/configUtils'); const logging = require('@tryghost/logging'); +const crypto = require('crypto'); const images = []; let agent, frontendAgent, ghostServer; @@ -49,17 +50,19 @@ const uploadImageRequest = ({fileContents, filename, contentType, ref}) => { * @param {string} options.filename * @param {string} options.contentType * @param {string} [options.expectedFileName] - * @param {string} [options.expectedOriginalFileName] * @param {string} [options.ref] * @param {boolean} [options.skipOriginal] * @returns */ -const uploadImageCheck = async ({path, filename, contentType, expectedFileName, expectedOriginalFileName, ref, skipOriginal}) => { +const uploadImageCheck = async ({path, filename, contentType, expectedFileName, ref, skipOriginal}) => { const fileContents = await fs.readFile(path); const {body} = await uploadImageRequest({fileContents, filename, contentType, ref}).expectStatus(201); + expectedFileName = expectedFileName || filename; + const parsedFileName = p.parse(expectedFileName); + const expectedFileNameRegex = `${parsedFileName.name}-\\w{16}${parsedFileName.ext}`; - assert.match(body.images[0].url, new RegExp(`${urlUtils.urlFor('home', true)}content/images/\\d+/\\d+/${expectedFileName}`)); + assert.match(body.images[0].url, new RegExp(`${urlUtils.urlFor('home', true)}content/images/\\d+/\\d+/${expectedFileNameRegex}`)); assert.equal(body.images[0].ref, ref === undefined ? null : ref); const relativePath = body.images[0].url.replace(urlUtils.urlFor('home', true), '/'); @@ -67,8 +70,11 @@ const uploadImageCheck = async ({path, filename, contentType, expectedFileName, images.push(filePath); // Get original image path - const originalFilePath = skipOriginal ? filePath : (expectedOriginalFileName ? filePath.replace(expectedFileName, expectedOriginalFileName) : imageTransform.generateOriginalImageName(filePath)); - images.push(originalFilePath); + let originalFilePath = filePath; + if (!skipOriginal) { + originalFilePath = imageTransform.generateOriginalImageName(filePath); + images.push(originalFilePath); + } // Check the image is saved to disk const saved = await fs.readFile(originalFilePath); @@ -242,20 +248,6 @@ describe('Images API', function () { await uploadImageCheck({path: originalFilePath, filename: 'loadingcat_square.gif', contentType: 'image/gif'}); }); - it('Will error when filename is too long', async function () { - const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); - const fileContents = await fs.readFile(originalFilePath); - const loggingStub = sinon.stub(logging, 'error'); - await uploadImageRequest({fileContents, filename: `${'a'.repeat(300)}.png`, contentType: 'image/png'}) - .expectStatus(400) - .matchBodySnapshot({ - errors: [{ - id: anyErrorId - }] - }); - sinon.assert.calledOnce(loggingStub); - }); - it('Can not upload a json file', async function () { const originalFilePath = p.join(__dirname, '/../../utils/fixtures/data/redirects.json'); const fileContents = await fs.readFile(originalFilePath); @@ -312,12 +304,21 @@ describe('Images API', function () { sinon.assert.calledOnce(loggingStub); }); + it('Can upload a file with a long name and the filename will be truncated to be under 253 bytes', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); + const ext = '.png'; + const hash = `-${crypto.randomBytes(8).toString('hex')}`; + const truncatedNameLength = 253 - hash.length - ext.length; + + await uploadImageCheck({path: originalFilePath, filename: `${'a'.repeat(300)}.png`, expectedFileName: `${'a'.repeat(truncatedNameLength)}.png`, contentType: 'image/png'}); + }); + it('Can upload multiple images with the same name', async function () { const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); const originalFilePath2 = p.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg'); await uploadImageCheck({path: originalFilePath, filename: 'a.png', contentType: 'image/png'}); - await uploadImageCheck({path: originalFilePath2, filename: 'a.png', contentType: 'image/png', expectedFileName: 'a-1.png', expectedOriginalFileName: 'a-1_o.png'}); + await uploadImageCheck({path: originalFilePath2, filename: 'a.png', contentType: 'image/png'}); }); it('Can upload image with number suffix', async function () { @@ -327,12 +328,12 @@ describe('Images API', function () { it('Trims _o suffix from uploaded files', async function () { const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); - await uploadImageCheck({path: originalFilePath, filename: 'a-3_o.png', contentType: 'image/png', expectedFileName: 'a-3.png', expectedOriginalFileName: 'a-3_o.png'}); + await uploadImageCheck({path: originalFilePath, filename: 'a-3_o.png', contentType: 'image/png', expectedFileName: 'a-3.png'}); }); it('Can use _o in uploaded file name, as long as it is not at the end', async function () { const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); - await uploadImageCheck({path: originalFilePath, filename: 'a_o-3.png', contentType: 'image/png', expectedFileName: 'a_o-3.png', expectedOriginalFileName: 'a_o-3_o.png'}); + await uploadImageCheck({path: originalFilePath, filename: 'a_o-3.png', contentType: 'image/png', expectedFileName: 'a_o-3.png'}); }); it('Can upload around midnight of month change', async function () { diff --git a/ghost/core/test/e2e-api/admin/media.test.js b/ghost/core/test/e2e-api/admin/media.test.js index 5f88cb489ea..f3ce379a0d5 100644 --- a/ghost/core/test/e2e-api/admin/media.test.js +++ b/ghost/core/test/e2e-api/admin/media.test.js @@ -38,8 +38,8 @@ describe('Media API', function () { .attach('thumbnail', path.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png')) .expect(201); - res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360.mp4`)); - res.body.media[0].thumbnail_url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360_thumb.png`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360-\\w{16}\\.mp4`)); + res.body.media[0].thumbnail_url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360_thumb-\\w{16}\\.png`)); res.body.media[0].ref.should.equal('https://ghost.org/sample_640x360.mp4'); media.push(res.body.media[0].url.replace(config.get('url'), '')); @@ -54,7 +54,7 @@ describe('Media API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/media/sample_640x360.webm')) .expect(201); - res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360.webm`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360-\\w{16}\\.webm`)); should(res.body.media[0].thumbnail_url).eql(null); res.body.media[0].ref.should.equal('https://ghost.org/sample_640x360.webm'); @@ -70,7 +70,7 @@ describe('Media API', function () { .attach('thumbnail', path.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png')) .expect(201); - res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360.ogv`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360-\\w{16}\\.ogv`)); res.body.media[0].ref.should.equal('https://ghost.org/sample_640x360.ogv'); media.push(res.body.media[0].url.replace(config.get('url'), '')); @@ -84,7 +84,7 @@ describe('Media API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/media/sample.mp3')) .expect(201); - res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample.mp3`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample-\\w{16}\\.mp3`)); res.body.media[0].ref.should.equal('audio_file_123'); media.push(res.body.media[0].url.replace(config.get('url'), '')); @@ -98,7 +98,7 @@ describe('Media API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/media/sample.m4a'), {filename: 'audio-mp4.m4a', contentType: 'audio/mp4'}) .expect(201); - res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/audio-mp4.m4a`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/audio-mp4-\\w{16}\\.m4a`)); res.body.media[0].ref.should.equal('audio_file_mp4'); media.push(res.body.media[0].url.replace(config.get('url'), '')); @@ -112,7 +112,7 @@ describe('Media API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/media/sample.m4a'), {filename: 'audio-x-m4a.m4a', contentType: 'audio/x-m4a'}) .expect(201); - res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/audio-x-m4a.m4a`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/audio-x-m4a-\\w{16}\\.m4a`)); res.body.media[0].ref.should.equal('audio_file_x_m4a'); media.push(res.body.media[0].url.replace(config.get('url'), '')); @@ -188,7 +188,7 @@ describe('Media API', function () { .attach('thumbnail', path.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png')) .expect(201); - res.body.media[0].ref.should.equal('https://ghost.org/sample_640x360.mp4'); + res.body.media[0].ref.should.match(/https:\/\/ghost\.org\/sample_640x360.mp4/); media.push(res.body.media[0].url.replace(config.get('url'), '')); media.push(res.body.media[0].thumbnail_url.replace(config.get('url'), '')); @@ -201,8 +201,8 @@ describe('Media API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg')) .expect(200); - const thumbnailUrl = res.body.media[0].url.replace('.mp4', '_thumb.jpg'); - thumbnailRes.body.media[0].url.should.equal(thumbnailUrl); + const mediaUrlWithoutExt = res.body.media[0].url.replace('.mp4', ''); + thumbnailRes.body.media[0].url.should.match(new RegExp(`${mediaUrlWithoutExt}_thumb-\\w{16}\\.jpg`)); thumbnailRes.body.media[0].ref.should.equal('updated_thumbnail'); media.push(thumbnailRes.body.media[0].url.replace(config.get('url'), '')); }); @@ -225,8 +225,8 @@ describe('Media API', function () { .attach('file', path.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg')) .expect(200); - const thumbnailUrl = res.body.media[0].url.replace('.mp4', '_thumb.jpg'); - thumbnailRes.body.media[0].url.should.equal(thumbnailUrl); + const mediaUrlWithoutExt = res.body.media[0].url.replace('.mp4', ''); + thumbnailRes.body.media[0].url.should.match(new RegExp(`${mediaUrlWithoutExt}_thumb-\\w{16}\\.jpg`)); thumbnailRes.body.media[0].ref.should.equal('updated_thumbnail_2'); media.push(thumbnailRes.body.media[0].url.replace(config.get('url'), '')); diff --git a/ghost/core/test/unit/server/adapters/storage/LocalImagesStorage.test.js b/ghost/core/test/unit/server/adapters/storage/LocalImagesStorage.test.js index 702ab103c3c..4ba63dd39c5 100644 --- a/ghost/core/test/unit/server/adapters/storage/LocalImagesStorage.test.js +++ b/ghost/core/test/unit/server/adapters/storage/LocalImagesStorage.test.js @@ -1,11 +1,11 @@ const errors = require('@tryghost/errors'); -const should = require('should'); const sinon = require('sinon'); const fs = require('fs-extra'); const moment = require('moment'); const path = require('path'); const LocalImagesStorage = require('../../../../../core/server/adapters/storage/LocalImagesStorage'); const configUtils = require('../../../../utils/configUtils'); +const assert = require('assert').strict; describe('Local Images Storage', function () { let image; @@ -47,96 +47,51 @@ describe('Local Images Storage', function () { fakeDate(9, 2013); }); - it('should send correct path to image when date is in Sep 2013', function (done) { - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2013/09/IMAGE.jpg'); - - done(); - }).catch(done); + it('sends correct path to image when date is in Sep 2013', async function () { + const url = await localFileStore.save(image); + assert.match(url, /content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); }); - it('should send correct path to image when original file has spaces', function (done) { + it('sends correct path to image when original file has spaces', async function () { image.name = 'AN IMAGE.jpg'; - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2013/09/AN-IMAGE.jpg'); - - done(); - }).catch(done); + const url = await localFileStore.save(image); + assert.match(url, /content\/images\/2013\/09\/AN-IMAGE-\w{16}\.jpg/); }); - it('should allow "@" symbol to image for Apple hi-res (retina) modifier', function (done) { + it('allows "@" symbol to image for Apple hi-res (retina) modifier', async function () { image.name = 'photo@2x.jpg'; - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2013/09/photo@2x.jpg'); - - done(); - }).catch(done); + const url = await localFileStore.save(image); + assert.match(url, /content\/images\/2013\/09\/photo@2x-\w{16}\.jpg/); }); - it('should send correct path to image when date is in Jan 2014', function (done) { + it('sends correct path to image when date is in Jan 2014', async function () { fakeDate(1, 2014); - - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2014/01/IMAGE.jpg'); - - done(); - }).catch(done); + const url = await localFileStore.save(image); + assert.match(url, /content\/images\/2014\/01\/IMAGE-\w{16}\.jpg/); }); - it('should create month and year directory', function (done) { - localFileStore.save(image).then(function () { - fs.mkdirs.calledOnce.should.be.true(); - fs.mkdirs.args[0][0].should.equal(path.resolve('./content/images/2013/09')); - - done(); - }).catch(done); + it('creates month and year directory', async function () { + await localFileStore.save(image); + assert.equal(fs.mkdirs.calledOnce, true); + assert.equal(fs.mkdirs.args[0][0], path.resolve('./content/images/2013/09')); }); - it('should copy temp file to new location', function (done) { - localFileStore.save(image).then(function () { - fs.copy.calledOnce.should.be.true(); - fs.copy.args[0][0].should.equal('tmp/123456.jpg'); - fs.copy.args[0][1].should.equal(path.resolve('./content/images/2013/09/IMAGE.jpg')); + it('copies temp file to new location', async function () { + await localFileStore.save(image); + assert.equal(fs.copy.calledOnce, true); + assert.equal(fs.copy.args[0][0], 'tmp/123456.jpg'); - done(); - }).catch(done); + assert.match(fs.copy.args[0][1], /content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); }); - it('can upload two different images with the same name without overwriting the first', function (done) { - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE.jpg')).resolves(); - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-1.jpg')).rejects(); - - // if on windows need to setup with back slashes - // doesn't hurt for the test to cope with both - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE.jpg')).resolves(); - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE-1.jpg')).rejects(); + it('uploads two different images with the same name with different names', async function () { + const first = await localFileStore.save(image); + assert.match(first, /content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2013/09/IMAGE-1.jpg'); + const second = await localFileStore.save(image); + assert.match(second, /content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); - done(); - }).catch(done); - }); - - it('can upload five different images with the same name without overwriting the first', function (done) { - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE.jpg')).resolves(); - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-1.jpg')).resolves(); - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-2.jpg')).resolves(); - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-3.jpg')).resolves(); - fs.stat.withArgs(path.resolve('./content/images/2013/09/IMAGE-4.jpg')).rejects(); - - // windows setup - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE.jpg')).resolves(); - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE-1.jpg')).resolves(); - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE-2.jpg')).resolves(); - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE-3.jpg')).resolves(); - fs.stat.withArgs(path.resolve('.\\content\\images\\2013\\Sep\\IMAGE-4.jpg')).rejects(); - - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2013/09/IMAGE-4.jpg'); - - done(); - }).catch(done); + assert.notEqual(first, second); }); describe('read image', function () { @@ -145,61 +100,45 @@ describe('Local Images Storage', function () { localFileStore.storagePath = path.join(__dirname, '../../../../utils/fixtures/images/'); }); - it('success', function (done) { - localFileStore.read({path: 'ghost-logo.png'}) - .then(function (bytes) { - bytes.length.should.eql(8638); - done(); - }); + it('reads image', async function () { + const bytes = await localFileStore.read({path: 'ghost-logo.png'}); + assert.equal(bytes.length, 8638); }); - it('success (leading and trailing slashes)', function (done) { - localFileStore.read({path: '/ghost-logo.png/'}) - .then(function (bytes) { - bytes.length.should.eql(8638); - done(); - }); + it('reads image (leading and trailing slashes)', async function () { + const bytes = await localFileStore.read({path: '/ghost-logo.png/'}); + assert.equal(bytes.length, 8638); }); - it('image does not exist', function (done) { - localFileStore.read({path: 'does-not-exist.png'}) - .then(function () { - done(new Error('image should not exist')); - }) - .catch(function (err) { - (err instanceof errors.NotFoundError).should.eql(true); - err.code.should.eql('ENOENT'); - done(); - }); + it('returns error when image does not exist', async function () { + await assert.rejects( + localFileStore.read({path: 'does-not-exist.png'}), + errors.NotFoundError, + 'Expected error to be thrown' + ); }); }); describe('validate extentions', function () { - it('name contains a .\d as extension', function (done) { - localFileStore.save({ - name: 'test-1.1.1' - }).then(function (url) { - should.exist(url.match(/test-1.1.1/)); - done(); - }).catch(done); - }); - - it('name contains a .zip as extension', function (done) { - localFileStore.save({ + it('saves image with .zip as extension', async function () { + const url = await localFileStore.save({ name: 'test-1.1.1.zip' - }).then(function (url) { - should.exist(url.match(/test-1.1.1.zip/)); - done(); - }).catch(done); + }); + assert.match(url, /test-1.1.1-\w{16}\.zip/); }); - it('name contains a .jpeg as extension', function (done) { - localFileStore.save({ + it('saves image with .jpeg as extension', async function () { + const url = await localFileStore.save({ name: 'test-1.1.1.jpeg' - }).then(function (url) { - should.exist(url.match(/test-1.1.1.jpeg/)); - done(); - }).catch(done); + }); + assert.match(url, /test-1.1.1-\w{16}\.jpeg/); + }); + + it('saves image but ignores invalid extension .0', async function () { + const url = await localFileStore.save({ + name: 'test-1.1.1.0' + }); + assert.match(url, /test-1.1.1.0-\w{16}/); }); }); @@ -209,12 +148,9 @@ describe('Local Images Storage', function () { configUtils.set('paths:contentPath', configPaths.appRoot + '/var/ghostcms'); }); - it('should send the correct path to image', function (done) { - localFileStore.save(image).then(function (url) { - url.should.equal('/content/images/2013/09/IMAGE.jpg'); - - done(); - }).catch(done); + it('sends the correct path to image', async function () { + const url = await localFileStore.save(image); + assert.match(url, /\/content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); }); }); @@ -231,23 +167,20 @@ describe('Local Images Storage', function () { path.sep = truePathSep; }); - it('should return url in proper format for windows', function (done) { + it('returns url in proper format for windows', async function () { path.sep = '\\'; path.join.returns('content\\images\\2013\\09\\IMAGE.jpg'); - localFileStore.save(image).then(function (url) { - if (truePathSep === '\\') { - url.should.equal('/content/images/2013/09/IMAGE.jpg'); - } else { - // if this unit test is run on an OS that uses forward slash separators, - // localfilesystem.save() will use a path.relative() call on - // one path with backslash separators and one path with forward - // slashes and it returns a path that needs to be normalized - path.normalize(url).should.equal('/content/images/2013/09/IMAGE.jpg'); - } - - done(); - }).catch(done); + const url = await localFileStore.save(image); + if (truePathSep === '\\') { + assert.equal(url, '/content/images/2013/09/IMAGE.jpg'); + } else { + // if this unit test is run on an OS that uses forward slash separators, + // localfilesystem.save() will use a path.relative() call on + // one path with backslash separators and one path with forward + // slashes and it returns a path that needs to be normalized + assert.equal(path.normalize(url), '/content/images/2013/09/IMAGE.jpg'); + } }); }); }); diff --git a/ghost/oembed-service/lib/OEmbedService.js b/ghost/oembed-service/lib/OEmbedService.js index a63363f569a..746af637030 100644 --- a/ghost/oembed-service/lib/OEmbedService.js +++ b/ghost/oembed-service/lib/OEmbedService.js @@ -134,13 +134,13 @@ class OEmbedService { */ async fetchImageBuffer(imageUrl) { const response = await fetch(imageUrl); - + if (!response.ok) { throw Error(`Failed to fetch image: ${response.statusText}`); } const arrayBuffer = await response.arrayBuffer(); - + const buffer = Buffer.from(arrayBuffer); return buffer; } @@ -162,9 +162,9 @@ class OEmbedService { let name; if (ext) { - name = store.getSanitizedFileName(path.basename(fileName, ext)); + name = store.sanitizeFileName(path.basename(fileName, ext)); } else { - name = store.getSanitizedFileName(path.basename(fileName)); + name = store.sanitizeFileName(path.basename(fileName)); } let targetDir = path.join(this.config.getContentPath('images'), imageType); diff --git a/yarn.lock b/yarn.lock index a915713b6c1..4401946ba58 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18861,10 +18861,10 @@ getpass@^0.1.1: dependencies: assert-plus "^1.0.0" -ghost-storage-base@1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/ghost-storage-base/-/ghost-storage-base-1.0.0.tgz#931289d310ad59fc80e2be01a81235cc3a76e75a" - integrity sha512-qIW6pny/wWKjrbRmXVNis9i7856AMR5/NZmnLTrKbA0KIEnA9K/fhkj7ISnSyTYfBv17sFsC23eJfvj6dDgZrQ== +ghost-storage-base@1.1.1: + version "1.1.1" + resolved "https://registry.yarnpkg.com/ghost-storage-base/-/ghost-storage-base-1.1.1.tgz#63caec4af9cb2f5cd0271cc87bf85cbadd135de8" + integrity sha512-MRokcZctPKO/Oonn2W55dYNZRPn75lBoSdoOc1BtwL7wm/Sq/Qx7ovx1H5seZhCReFs8QOeUXvX9dXuguBSnnQ== dependencies: moment "2.27.0"