From 559c7ead7c6f2923698db06974fa16340a10f6db Mon Sep 17 00:00:00 2001 From: Sag Date: Tue, 4 Feb 2025 21:46:03 +0700 Subject: [PATCH 1/2] Revert "Made names for uploaded files more secure and resilient to filesystem limits (#22055)" (#22108) ref https://linear.app/ghost/issue/ONC-758 ref https://linear.app/ghost/issue/ENG-1260 ref https://linear.app/ghost/issue/ENG-1859 - this reverts commit 4e4651ae071ff2b0a93b1724a761f6b72e5fb1f6 - changes in the reverted commit are blocking customers migrating from other platforms from using the universal import feature --- .../adapters/storage/LocalStorageBase.js | 29 +-- .../core/core/server/api/endpoints/images.js | 3 +- .../core/server/services/themes/storage.js | 6 +- ghost/core/package.json | 2 +- ghost/core/test/e2e-api/admin/files.test.js | 2 +- ghost/core/test/e2e-api/admin/images.test.js | 45 ++-- ghost/core/test/e2e-api/admin/media.test.js | 24 +- .../storage/LocalImagesStorage.test.js | 205 ++++++++++++------ ghost/oembed-service/lib/OEmbedService.js | 8 +- yarn.lock | 8 +- 10 files changed, 194 insertions(+), 138 deletions(-) diff --git a/ghost/core/core/server/adapters/storage/LocalStorageBase.js b/ghost/core/core/server/adapters/storage/LocalStorageBase.js index fa6609aa89..915a30a81f 100644 --- a/ghost/core/core/server/adapters/storage/LocalStorageBase.js +++ b/ghost/core/core/server/adapters/storage/LocalStorageBase.js @@ -43,30 +43,23 @@ 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 {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 {StorageBase.Image} file * @param {String} targetDir * @returns {Promise} */ async save(file, 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; + let targetFilename; - // 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); - } + // 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); try { - await fs.mkdirs(directory); - await fs.copy(originalFilePath, targetFilePath); + await fs.copy(file.path, targetFilename); } catch (err) { if (err.code === 'ENAMETOOLONG') { throw new errors.BadRequestError({err}); @@ -81,7 +74,7 @@ class LocalStorageBase extends StorageBase { urlUtils.urlJoin('/', urlUtils.getSubdir(), this.staticFileURLPrefix, - path.relative(this.storagePath, targetFilePath)) + path.relative(this.storagePath, targetFilename)) ).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 ce89d34b14..b95dedc0ab 100644 --- a/ghost/core/core/server/api/endpoints/images.js +++ b/ghost/core/core/server/api/endpoints/images.js @@ -71,8 +71,7 @@ const controller = { await store.save({ ...frame.file, path: originalPath, - name: imageTransform.generateOriginalImageName(processedImageName), - keepOriginalName: true // Don't generate a new filename on save + name: imageTransform.generateOriginalImageName(processedImageName) }, processedImageDir); return processedImageUrl; diff --git a/ghost/core/core/server/services/themes/storage.js b/ghost/core/core/server/services/themes/storage.js index 7e7f45234d..f087e37e13 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().sanitizeFileName(zip.name.split('.zip')[0]); + const themeName = getStorage().getSanitizedFileName(zip.name.split('.zip')[0]); const backupName = `${themeName}_${ObjectID()}`; // check if zip name matches one of the default themes @@ -63,7 +63,6 @@ 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'); @@ -74,8 +73,7 @@ module.exports = { // CASE: store extracted theme await getStorage().save({ name: themeName, - path: checkedTheme.path, - keepOriginalName: true + path: checkedTheme.path }); // 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 a27f05efe1..c881993522 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.1.1", + "ghost-storage-base": "1.0.0", "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 a5f0f11c57..300e0294a9 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-\\w{16}\\.gif`)); + res.body.files[0].url.should.match(new RegExp(`${config.get('url')}/content/files/\\d+/\\d+/loadingcat_square.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 e3dbbfde66..db3679a350 100644 --- a/ghost/core/test/e2e-api/admin/images.test.js +++ b/ghost/core/test/e2e-api/admin/images.test.js @@ -13,7 +13,6 @@ 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; @@ -50,19 +49,17 @@ 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, ref, skipOriginal}) => { +const uploadImageCheck = async ({path, filename, contentType, expectedFileName, expectedOriginalFileName, 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+/${expectedFileNameRegex}`)); + assert.match(body.images[0].url, new RegExp(`${urlUtils.urlFor('home', true)}content/images/\\d+/\\d+/${expectedFileName}`)); assert.equal(body.images[0].ref, ref === undefined ? null : ref); const relativePath = body.images[0].url.replace(urlUtils.urlFor('home', true), '/'); @@ -70,11 +67,8 @@ const uploadImageCheck = async ({path, filename, contentType, expectedFileName, images.push(filePath); // Get original image path - let originalFilePath = filePath; - if (!skipOriginal) { - originalFilePath = imageTransform.generateOriginalImageName(filePath); - images.push(originalFilePath); - } + const originalFilePath = skipOriginal ? filePath : (expectedOriginalFileName ? filePath.replace(expectedFileName, expectedOriginalFileName) : imageTransform.generateOriginalImageName(filePath)); + images.push(originalFilePath); // Check the image is saved to disk const saved = await fs.readFile(originalFilePath); @@ -248,6 +242,20 @@ 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); @@ -304,21 +312,12 @@ 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'}); + await uploadImageCheck({path: originalFilePath2, filename: 'a.png', contentType: 'image/png', expectedFileName: 'a-1.png', expectedOriginalFileName: 'a-1_o.png'}); }); it('Can upload image with number suffix', async function () { @@ -328,12 +327,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'}); + await uploadImageCheck({path: originalFilePath, filename: 'a-3_o.png', contentType: 'image/png', expectedFileName: 'a-3.png', expectedOriginalFileName: 'a-3_o.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'}); + await uploadImageCheck({path: originalFilePath, filename: 'a_o-3.png', contentType: 'image/png', expectedFileName: 'a_o-3.png', expectedOriginalFileName: 'a_o-3_o.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 f3ce379a0d..5f88cb489e 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-\\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].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].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-\\w{16}\\.webm`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360.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-\\w{16}\\.ogv`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample_640x360.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-\\w{16}\\.mp3`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/sample.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-\\w{16}\\.m4a`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/audio-mp4.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-\\w{16}\\.m4a`)); + res.body.media[0].url.should.match(new RegExp(`${config.get('url')}/content/media/\\d+/\\d+/audio-x-m4a.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.match(/https:\/\/ghost\.org\/sample_640x360.mp4/); + res.body.media[0].ref.should.equal('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 mediaUrlWithoutExt = res.body.media[0].url.replace('.mp4', ''); - thumbnailRes.body.media[0].url.should.match(new RegExp(`${mediaUrlWithoutExt}_thumb-\\w{16}\\.jpg`)); + const thumbnailUrl = res.body.media[0].url.replace('.mp4', '_thumb.jpg'); + thumbnailRes.body.media[0].url.should.equal(thumbnailUrl); 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 mediaUrlWithoutExt = res.body.media[0].url.replace('.mp4', ''); - thumbnailRes.body.media[0].url.should.match(new RegExp(`${mediaUrlWithoutExt}_thumb-\\w{16}\\.jpg`)); + const thumbnailUrl = res.body.media[0].url.replace('.mp4', '_thumb.jpg'); + thumbnailRes.body.media[0].url.should.equal(thumbnailUrl); 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 4ba63dd39c..702ab103c3 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,51 +47,96 @@ describe('Local Images Storage', function () { fakeDate(9, 2013); }); - 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 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 original file has spaces', async function () { + it('should send correct path to image when original file has spaces', function (done) { image.name = 'AN IMAGE.jpg'; - const url = await localFileStore.save(image); - assert.match(url, /content\/images\/2013\/09\/AN-IMAGE-\w{16}\.jpg/); + localFileStore.save(image).then(function (url) { + url.should.equal('/content/images/2013/09/AN-IMAGE.jpg'); + + done(); + }).catch(done); }); - it('allows "@" symbol to image for Apple hi-res (retina) modifier', async function () { + it('should allow "@" symbol to image for Apple hi-res (retina) modifier', function (done) { image.name = 'photo@2x.jpg'; - const url = await localFileStore.save(image); - assert.match(url, /content\/images\/2013\/09\/photo@2x-\w{16}\.jpg/); + localFileStore.save(image).then(function (url) { + url.should.equal('/content/images/2013/09/photo@2x.jpg'); + + done(); + }).catch(done); }); - it('sends correct path to image when date is in Jan 2014', async function () { + it('should send correct path to image when date is in Jan 2014', function (done) { fakeDate(1, 2014); - const url = await localFileStore.save(image); - assert.match(url, /content\/images\/2014\/01\/IMAGE-\w{16}\.jpg/); + + localFileStore.save(image).then(function (url) { + url.should.equal('/content/images/2014/01/IMAGE.jpg'); + + 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 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('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'); + 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')); - assert.match(fs.copy.args[0][1], /content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); + done(); + }).catch(done); }); - 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/); + 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(); - const second = await localFileStore.save(image); - assert.match(second, /content\/images\/2013\/09\/IMAGE-\w{16}\.jpg/); + // 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(); - assert.notEqual(first, second); + localFileStore.save(image).then(function (url) { + url.should.equal('/content/images/2013/09/IMAGE-1.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); }); describe('read image', function () { @@ -100,45 +145,61 @@ describe('Local Images Storage', function () { localFileStore.storagePath = path.join(__dirname, '../../../../utils/fixtures/images/'); }); - it('reads image', async function () { - const bytes = await localFileStore.read({path: 'ghost-logo.png'}); - assert.equal(bytes.length, 8638); + it('success', 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('success (leading and trailing slashes)', function (done) { + localFileStore.read({path: '/ghost-logo.png/'}) + .then(function (bytes) { + bytes.length.should.eql(8638); + 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' - ); + 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(); + }); }); }); describe('validate extentions', function () { - it('saves image with .zip as extension', async function () { - const url = await localFileStore.save({ + 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({ name: 'test-1.1.1.zip' - }); - assert.match(url, /test-1.1.1-\w{16}\.zip/); + }).then(function (url) { + should.exist(url.match(/test-1.1.1.zip/)); + done(); + }).catch(done); }); - it('saves image with .jpeg as extension', async function () { - const url = await localFileStore.save({ + it('name contains a .jpeg as extension', function (done) { + localFileStore.save({ name: 'test-1.1.1.jpeg' - }); - 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}/); + }).then(function (url) { + should.exist(url.match(/test-1.1.1.jpeg/)); + done(); + }).catch(done); }); }); @@ -148,9 +209,12 @@ describe('Local Images Storage', function () { configUtils.set('paths:contentPath', configPaths.appRoot + '/var/ghostcms'); }); - 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/); + 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); }); }); @@ -167,20 +231,23 @@ describe('Local Images Storage', function () { path.sep = truePathSep; }); - it('returns url in proper format for windows', async function () { + it('should return url in proper format for windows', function (done) { path.sep = '\\'; path.join.returns('content\\images\\2013\\09\\IMAGE.jpg'); - 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'); - } + 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); }); }); }); diff --git a/ghost/oembed-service/lib/OEmbedService.js b/ghost/oembed-service/lib/OEmbedService.js index 746af63703..a63363f569 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.sanitizeFileName(path.basename(fileName, ext)); + name = store.getSanitizedFileName(path.basename(fileName, ext)); } else { - name = store.sanitizeFileName(path.basename(fileName)); + name = store.getSanitizedFileName(path.basename(fileName)); } let targetDir = path.join(this.config.getContentPath('images'), imageType); diff --git a/yarn.lock b/yarn.lock index c7973088ed..f7ebf6aa8c 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.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== +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== dependencies: moment "2.27.0" From 0b50f330db7f15d2e3e83f02d1c897057c246183 Mon Sep 17 00:00:00 2001 From: Ghost CI <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 4 Feb 2025 15:18:16 +0000 Subject: [PATCH 2/2] v5.109.2 --- ghost/admin/package.json | 2 +- ghost/core/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ghost/admin/package.json b/ghost/admin/package.json index eb5d6fa2d3..1f520be6c4 100644 --- a/ghost/admin/package.json +++ b/ghost/admin/package.json @@ -1,6 +1,6 @@ { "name": "ghost-admin", - "version": "5.109.1", + "version": "5.109.2", "description": "Ember.js admin client for Ghost", "author": "Ghost Foundation", "homepage": "http://ghost.org", diff --git a/ghost/core/package.json b/ghost/core/package.json index c881993522..18b4f8559d 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "5.109.1", + "version": "5.109.2", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org",