diff --git a/ghost/core/core/frontend/services/routing/router-manager.js b/ghost/core/core/frontend/services/routing/router-manager.js index 261b2e92aa..7960be6117 100644 --- a/ghost/core/core/frontend/services/routing/router-manager.js +++ b/ghost/core/core/frontend/services/routing/router-manager.js @@ -177,7 +177,7 @@ class RouterManager { // NOTE: timezone change only affects the collection router with dated permalinks const collectionRouter = this.registry.getRouterByName('CollectionRouter'); - if (collectionRouter.getPermalinks().getValue().match(/:year|:month|:day/)) { + if (collectionRouter && collectionRouter.getPermalinks().getValue().match(/:year|:month|:day/)) { debug('handleTimezoneEdit: trigger regeneration'); this.urlService.onRouterUpdated(collectionRouter.identifier); diff --git a/ghost/core/core/frontend/web/middleware/handle-image-sizes.js b/ghost/core/core/frontend/web/middleware/handle-image-sizes.js index 1c49a87315..edc5ba857f 100644 --- a/ghost/core/core/frontend/web/middleware/handle-image-sizes.js +++ b/ghost/core/core/frontend/web/middleware/handle-image-sizes.js @@ -5,6 +5,7 @@ const imageTransform = require('@tryghost/image-transform'); const storage = require('../../../server/adapters/storage'); const activeTheme = require('../../services/theme-engine/active'); const config = require('../../../shared/config'); +const {imageSize} = require('../../../server/lib/image'); const SIZE_PATH_REGEX = /^\/size\/([^/]+)\//; const FORMAT_PATH_REGEX = /^\/format\/([^./]+)\//; @@ -21,7 +22,7 @@ module.exports = function (req, res, next) { } const requestedDimension = req.url.match(SIZE_PATH_REGEX)[1]; - + // Note that we don't use sizeImageDir because we need to keep the trailing slash let imagePath = req.url.replace(`/size/${requestedDimension}`, ''); @@ -39,7 +40,7 @@ module.exports = function (req, res, next) { // We need to keep the first slash here let url = req.originalUrl .replace(`/size/${requestedDimension}`, ''); - + if (format) { url = url.replace(`/format/${format}`, ''); } @@ -81,7 +82,7 @@ module.exports = function (req, res, next) { return redirectToOriginal(); } - if (format) { + if (format) { // CASE: When formatting, we need to check if the imageTransform package supports this specific format if (!imageTransform.canTransformToFormat(format)) { // transform not supported @@ -89,7 +90,7 @@ module.exports = function (req, res, next) { } } - // CASE: when transforming is supported, we need to check if it is desired + // CASE: when transforming is supported, we need to check if it is desired // (e.g. it is not desired to resize SVGs when not formatting them to a different type) if (!format && !imageTransform.shouldResizeFileExtension(requestUrlFileExtension)) { return redirectToOriginal(); @@ -111,23 +112,7 @@ module.exports = function (req, res, next) { return; } - const {dir, name, ext} = path.parse(imagePath); - const [imageNameMatched, imageName, imageNumber] = name.match(/^(.+?)(-\d+)?$/) || [null]; - - if (!imageNameMatched) { - // CASE: Image name does not contain any characters? - // RESULT: Hand off to `next()` which will 404 - return; - } - const unoptimizedImagePath = path.join(dir, `${imageName}_o${imageNumber || ''}${ext}`); - - return storageInstance.exists(unoptimizedImagePath) - .then((unoptimizedImageExists) => { - if (unoptimizedImageExists) { - return unoptimizedImagePath; - } - return imagePath; - }) + return imageSize.getOriginalImagePath(imagePath) .then((storagePath) => { return storageInstance.read({path: storagePath}); }) diff --git a/ghost/core/core/server/adapters/storage/LocalStorageBase.js b/ghost/core/core/server/adapters/storage/LocalStorageBase.js index bc2969a19b..6145011f0a 100644 --- a/ghost/core/core/server/adapters/storage/LocalStorageBase.js +++ b/ghost/core/core/server/adapters/storage/LocalStorageBase.js @@ -83,9 +83,19 @@ class LocalStorageBase extends StorageBase { urlToPath(url) { let filePath; - if (url.match(this.staticFileUrl)) { + const prefix = urlUtils.urlJoin('/', + urlUtils.getSubdir(), + this.staticFileURLPrefix + ); + + if (url.startsWith(this.staticFileUrl)) { + // CASE: full path that includes the site url filePath = url.replace(this.staticFileUrl, ''); filePath = path.join(this.storagePath, filePath); + } else if (url.startsWith(prefix)) { + // CASE: The result of the save method doesn't include the site url. So we need to handle this case. + filePath = url.replace(prefix, ''); + filePath = path.join(this.storagePath, filePath); } else { throw new errors.IncorrectUsageError({ message: tpl(messages.invalidUrlParameter, {url}) diff --git a/ghost/core/core/server/api/endpoints/images.js b/ghost/core/core/server/api/endpoints/images.js index 0853b61889..e303776119 100644 --- a/ghost/core/core/server/api/endpoints/images.js +++ b/ghost/core/core/server/api/endpoints/images.js @@ -1,19 +1,60 @@ -const Promise = require('bluebird'); +/* eslint-disable ghost/ghost-custom/max-api-complexity */ const storage = require('../../adapters/storage'); +const imageTransform = require('@tryghost/image-transform'); +const config = require('../../../shared/config'); +const path = require('path'); module.exports = { docName: 'images', upload: { statusCode: 201, permissions: false, - query(frame) { + async query(frame) { const store = storage.getStorage('images'); - if (frame.files) { - return Promise - .map(frame.files, file => store.save(file)) - .then(paths => paths[0]); + // Normalize + const imageOptimizationOptions = config.get('imageOptimization'); + + // Trim _o from file name (not allowed suffix) + frame.file.name = frame.file.name.replace(/_o(\.\w+?)$/, '$1'); + + // CASE: image transform is not capable of transforming file (e.g. .gif) + if (imageTransform.shouldResizeFileExtension(frame.file.ext) && imageOptimizationOptions.resize) { + const out = `${frame.file.path}_processed`; + const originalPath = frame.file.path; + + const options = Object.assign({ + in: originalPath, + out, + ext: frame.file.ext, + width: config.get('imageOptimization:defaultMaxWidth') + }, imageOptimizationOptions); + + await imageTransform.resizeFromPath(options); + + // Store the processed/optimized image + const processedImageUrl = await store.save({ + ...frame.file, + path: out + }); + const processedImagePath = store.urlToPath(processedImageUrl); + + // Get the path and name of the processed image + // We want to store the original image on the same name + _o + // So we need to wait for the first store to finish before generating the name of the original image + const processedImageName = path.basename(processedImagePath); + const processedImageDir = path.dirname(processedImagePath); + + // Store the original image + await store.save({ + ...frame.file, + path: originalPath, + name: imageTransform.generateOriginalImageName(processedImageName) + }, processedImageDir); + + return processedImageUrl; } + return store.save(frame.file); } } diff --git a/ghost/core/core/server/lib/image/image-size.js b/ghost/core/core/server/lib/image/image-size.js index e4858c7c47..91b8f32c1f 100644 --- a/ghost/core/core/server/lib/image/image-size.js +++ b/ghost/core/core/server/lib/image/image-size.js @@ -270,18 +270,36 @@ class ImageSize { }); } - async getOriginalImageSizeFromStoragePath(imagePath) { + /** + * Returns the path of the original image for a given image path (we always store the original image in a separate file, suffixed with _o, while we store a resized version of the image on the original name) + * TODO: Preferrably we want to move this to a separate image utils package. Currently not really a good place to put it in image lib. + */ + async getOriginalImagePath(imagePath) { const {dir, name, ext} = path.parse(imagePath); + const storageInstance = this.storage.getStorage('images'); + + const preferredUnoptimizedImagePath = path.join(dir, `${name}_o${ext}`); + const preferredUnoptimizedImagePathExists = await storageInstance.exists(preferredUnoptimizedImagePath); + if (preferredUnoptimizedImagePathExists) { + return preferredUnoptimizedImagePath; + } + + // Legacy format did some magic with the numbers that could cause bugs. We still need to support it for old files. + // refs https://github.com/TryGhost/Team/issues/481 const [imageNameMatched, imageName, imageNumber] = name.match(/^(.+?)(-\d+)?$/) || [null]; if (!imageNameMatched) { - return this.getImageSizeFromStoragePath(imagePath); + return imagePath; } - const originalImagePath = path.join(dir, `${imageName}_o${imageNumber || ''}${ext}`); - const originalImageExists = await this.storage.getStorage('images').exists(originalImagePath); + const legacyOriginalImagePath = path.join(dir, `${imageName}_o${imageNumber || ''}${ext}`); + const legacyOriginalImageExists = await storageInstance.exists(legacyOriginalImagePath); - return this.getImageSizeFromStoragePath(originalImageExists ? originalImagePath : imagePath); + return legacyOriginalImageExists ? legacyOriginalImagePath : imagePath; + } + + async getOriginalImageSizeFromStoragePath(imagePath) { + return this.getImageSizeFromStoragePath(await this.getOriginalImagePath(imagePath)); } _getPathFromUrl(imageUrl) { diff --git a/ghost/core/core/server/web/api/endpoints/admin/routes.js b/ghost/core/core/server/web/api/endpoints/admin/routes.js index 9e2f4c1992..50f7e28bd5 100644 --- a/ghost/core/core/server/web/api/endpoints/admin/routes.js +++ b/ghost/core/core/server/web/api/endpoints/admin/routes.js @@ -240,7 +240,6 @@ module.exports = function apiRoutes() { mw.authAdminApi, apiMw.upload.single('file'), apiMw.upload.validation({type: 'images'}), - apiMw.normalizeImage, http(api.images.upload) ); diff --git a/ghost/core/core/server/web/api/middleware/index.js b/ghost/core/core/server/web/api/middleware/index.js index 6fa73fe121..91b438796c 100644 --- a/ghost/core/core/server/web/api/middleware/index.js +++ b/ghost/core/core/server/web/api/middleware/index.js @@ -1,6 +1,5 @@ module.exports = { cors: require('./cors'), - normalizeImage: require('./normalize-image'), updateUserLastSeen: require('./update-user-last-seen'), upload: require('./upload'), versionMatch: require('./version-match') diff --git a/ghost/core/core/server/web/api/middleware/normalize-image.js b/ghost/core/core/server/web/api/middleware/normalize-image.js deleted file mode 100644 index ec3b4708fb..0000000000 --- a/ghost/core/core/server/web/api/middleware/normalize-image.js +++ /dev/null @@ -1,42 +0,0 @@ -const cloneDeep = require('lodash/cloneDeep'); -const config = require('../../../../shared/config'); -const logging = require('@tryghost/logging'); -const imageTransform = require('@tryghost/image-transform'); - -module.exports = function normalize(req, res, next) { - const imageOptimizationOptions = config.get('imageOptimization'); - - // CASE: image transform is not capable of transforming file (e.g. .gif) - if (!imageTransform.shouldResizeFileExtension(req.file.ext) || !imageOptimizationOptions.resize) { - return next(); - } - - const out = `${req.file.path}_processed`; - const originalPath = req.file.path; - - const options = Object.assign({ - in: originalPath, - out, - ext: req.file.ext, - width: config.get('imageOptimization:defaultMaxWidth') - }, imageOptimizationOptions); - - imageTransform.resizeFromPath(options) - .then(() => { - req.files = []; - - // CASE: push the processed/optimized image - req.files.push(Object.assign(req.file, {path: out})); - - // CASE: push original image, we keep a copy of it - const newName = imageTransform.generateOriginalImageName(req.file.name); - req.files.push(Object.assign(cloneDeep(req.file), {path: originalPath, name: newName})); - - next(); - }) - .catch((err) => { - err.context = `${req.file.name} / ${req.file.type}`; - logging.error(err); - next(); - }); -}; diff --git a/ghost/core/package.json b/ghost/core/package.json index 5885e78390..47fcd4ef96 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -212,6 +212,7 @@ "eslint": "8.32.0", "expect": "^29.0.0", "html-validate": "7.13.1", + "form-data": "^4.0.0", "inquirer": "8.2.5", "jwks-rsa": "3.0.1", "mocha": "10.2.0", diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap new file mode 100644 index 0000000000..859e570433 --- /dev/null +++ b/ghost/core/test/e2e-api/admin/__snapshots__/images.test.js.snap @@ -0,0 +1,73 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Images API Can not upload a file without extension 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": null, + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Please select a valid image.", + "property": null, + "type": "UnsupportedMediaTypeError", + }, + ], +} +`; + +exports[`Images API Can not upload a json file 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": null, + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Please select a valid image.", + "property": null, + "type": "UnsupportedMediaTypeError", + }, + ], +} +`; + +exports[`Images API Can not upload a json file with image file extension 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": null, + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Please select a valid image.", + "property": null, + "type": "UnsupportedMediaTypeError", + }, + ], +} +`; + +exports[`Images API Can not upload a json file with image mime type 1: [body] 1`] = ` +Object { + "errors": Array [ + Object { + "code": null, + "context": null, + "details": null, + "ghostErrorCode": null, + "help": null, + "id": StringMatching /\\[a-f0-9\\]\\{8\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{4\\}-\\[a-f0-9\\]\\{12\\}/, + "message": "Please select a valid image.", + "property": null, + "type": "UnsupportedMediaTypeError", + }, + ], +} +`; diff --git a/ghost/core/test/e2e-api/admin/images.test.js b/ghost/core/test/e2e-api/admin/images.test.js index 426682fe1d..96054017fb 100644 --- a/ghost/core/test/e2e-api/admin/images.test.js +++ b/ghost/core/test/e2e-api/admin/images.test.js @@ -1,87 +1,308 @@ -const path = require('path'); -const fs = require('fs-extra'); -const should = require('should'); -const supertest = require('supertest'); -const localUtils = require('./utils'); -const testUtils = require('../../utils'); +const {agentProvider, fixtureManager, matchers} = require('../../utils/e2e-framework'); +const FormData = require('form-data'); +const p = require('path'); +const {promises: fs, stat} = require('fs'); +const assert = require('assert'); const config = require('../../../core/shared/config'); +const urlUtils = require('../../../core/shared/url-utils'); +const imageTransform = require('@tryghost/image-transform'); +const sinon = require('sinon'); +const storage = require('../../../core/server/adapters/storage'); +const sleep = require('../../utils/sleep'); +const {anyErrorId} = matchers; +const {imageSize} = require('../../../core/server/lib/image'); +const configUtils = require('../../utils/configUtils'); + +const images = []; +let agent, frontendAgent, ghostServer; +/** + * + * @param {object} options + * @param {Buffer} options.fileContents + * @param {string} options.filename + * @param {string} options.contentType + * @param {string} [options.ref] + * @returns + */ +const uploadImageRequest = ({fileContents, filename, contentType, ref}) => { + const form = new FormData(); + form.append('file', fileContents, { + filename, + contentType + }); + + form.append('purpose', 'image'); + if (ref) { + form.append('ref', ref); + } + + return agent + .post('/images/upload/') + .body(form); +}; + +/** + * + * @param {object} options + * @param {string} options.path + * @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 fileContents = await fs.readFile(path); + const {body} = await uploadImageRequest({fileContents, filename, contentType, ref}).expectStatus(201); + expectedFileName = expectedFileName || filename; + + assert.match(body.images[0].url, new RegExp(`${urlUtils.urlFor('home', true)}content/images/\\d+/\\d+/${expectedFileName}`)); + assert.equal(body.images[0].ref, ref); + + const relativePath = body.images[0].url.replace(urlUtils.urlFor('home', true), '/'); + const filePath = config.getContentPath('images') + relativePath.replace('/content/images/', ''); + images.push(filePath); + + // Get original image path + 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); + assert.equal(saved.length, fileContents.length); + assert.deepEqual(saved, fileContents); + + const savedResized = await fs.readFile(filePath); + assert.ok(savedResized.length <= fileContents.length); // should always be smaller + + // Check the image is served in the frontend using the provided URL + const {body: data} = await frontendAgent + .get(relativePath) + //.expect('Content-Length', savedResized.length.toString()) // not working for SVG for some reason? + .expect('Content-Type', contentType) + .expect(200); + assert.equal(Buffer.from(data).length, savedResized.length); + + // Make sure we support w10 + configUtils.set('imageOptimization:contentImageSizes', { + w10: {width: 10}, + w1: {width: 1} + }); + + // Check if image resizing and formatting works + const resizedPath = relativePath.replace('/content/images/', '/content/images/size/w10/format/webp/'); + await frontendAgent + .get(resizedPath) + .expect('Content-Type', 'image/webp') + .expect(200); + + const resizedFilePath = filePath.replace('/images/', '/images/size/w10/format/webp/'); + const size = await imageSize.getImageSizeFromPath(resizedFilePath); + assert.equal(size.width, 10, 'Resized images should have a width that has actually changed'); + + if (!contentType.includes('svg')) { + // Check if image resizing without formatting works + const resizedPath2 = relativePath.replace('/content/images/', '/content/images/size/w1/'); + await frontendAgent + .get(resizedPath2) + .expect('Content-Type', contentType) + .expect(200); + + const resizedFilePath2 = filePath.replace('/images/', '/images/size/w1/'); + const size2 = await imageSize.getImageSizeFromPath(resizedFilePath2); + + // Note: we chose width 1 because the resized image bytes should be less or it is ignored + assert.equal(size2.width, 1, 'Resized images should have a width that has actually changed'); + } + + return { + relativePath, + filePath, + originalFilePath, + fileContents + }; +}; describe('Images API', function () { - const images = []; - let request; - before(async function () { - await localUtils.startGhost(); - request = supertest.agent(config.get('url')); - await localUtils.doAuth(request); + const agents = await agentProvider.getAgentsWithFrontend(); + agent = agents.adminAgent; + frontendAgent = agents.frontendAgent; + ghostServer = agents.ghostServer; + await fixtureManager.init(); + await agent.loginAsOwner(); }); after(function () { - images.forEach(function (image) { - fs.removeSync(config.get('paths').appRoot + image); - }); + configUtils.restore(); + ghostServer.stop(); + }); + + afterEach(async function () { + // Delete all images after each test + for (const image of images) { + try { + await fs.unlink(image); + } catch (e) { + // ignore + } + } + + // Clean + images.splice(0, images.length); + sinon.restore(); }); it('Can upload a png', async function () { - const res = await request.post(localUtils.API.getApiQuery('images/upload')) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .field('purpose', 'image') - .field('ref', 'https://ghost.org/ghost-logo.png') - .attach('file', path.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png')) - .expect(201); - - res.body.images[0].url.should.match(new RegExp(`${config.get('url')}/content/images/\\d+/\\d+/ghost-logo.png`)); - res.body.images[0].ref.should.equal('https://ghost.org/ghost-logo.png'); - images.push(res.body.images[0].url.replace(config.get('url'), '')); + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); + await uploadImageCheck({path: originalFilePath, filename: 'ghost-logo.png', contentType: 'image/png', ref: 'https://ghost.org/ghost-logo.png'}); }); it('Can upload a jpg', async function () { - const res = await request.post(localUtils.API.getApiQuery('images/upload')) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .attach('file', path.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg')) - .expect(201); - - res.body.images[0].url.should.match(new RegExp(`${config.get('url')}/content/images/\\d+/\\d+/ghosticon.jpg`)); - should(res.body.images[0].ref).equal(null); - - images.push(res.body.images[0].url.replace(config.get('url'), '')); + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg'); + await uploadImageCheck({path: originalFilePath, filename: 'ghosticon.jpg', contentType: 'image/jpeg'}); }); it('Can upload a gif', async function () { - const res = await request.post(localUtils.API.getApiQuery('images/upload')) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .attach('file', path.join(__dirname, '/../../utils/fixtures/images/loadingcat.gif')) - .expect(201); - - res.body.images[0].url.should.match(new RegExp(`${config.get('url')}/content/images/\\d+/\\d+/loadingcat.gif`)); - - images.push(res.body.images[0].url.replace(config.get('url'), '')); + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/loadingcat.gif'); + await uploadImageCheck({path: originalFilePath, filename: 'loadingcat.gif', contentType: 'image/gif'}); }); it('Can upload a webp', async function () { - const res = await request.post(localUtils.API.getApiQuery('images/upload')) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .attach('file', path.join(__dirname, '/../../utils/fixtures/images/ghosticon.webp')) - .expect(201); + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghosticon.webp'); + await uploadImageCheck({path: originalFilePath, filename: 'ghosticon.webp', contentType: 'image/webp'}); + }); - res.body.images[0].url.should.match(new RegExp(`${config.get('url')}/content/images/\\d+/\\d+/ghosticon.webp`)); - - images.push(res.body.images[0].url.replace(config.get('url'), '')); + it('Can upload a svg', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.svg'); + await uploadImageCheck({path: originalFilePath, filename: 'ghost.svg', contentType: 'image/svg+xml', skipOriginal: true}); }); it('Can upload a square profile image', async function () { - const res = await request.post(localUtils.API.getApiQuery('images/upload')) - .set('Origin', config.get('url')) - .expect('Content-Type', /json/) - .attach('file', path.join(__dirname, '/../../utils/fixtures/images/loadingcat_square.gif')) - .expect(201); + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/loadingcat_square.gif'); + await uploadImageCheck({path: originalFilePath, filename: 'loadingcat_square.gif', contentType: 'image/gif'}); + }); - res.body.images[0].url.should.match(new RegExp(`${config.get('url')}/content/images/\\d+/\\d+/loadingcat_square.gif`)); + 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); + await uploadImageRequest({fileContents, filename: 'redirects.json', contentType: 'application/json'}) + .expectStatus(415) + .matchBodySnapshot({ + errors: [{ + id: anyErrorId + }] + }); + }); - images.push(res.body.images[0].url.replace(config.get('url'), '')); + it('Can not upload a file without extension', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/data/redirects.json'); + const fileContents = await fs.readFile(originalFilePath); + await uploadImageRequest({fileContents, filename: 'redirects', contentType: 'image/png'}) + .expectStatus(415) + .matchBodySnapshot({ + errors: [{ + id: anyErrorId + }] + }); + }); + + it('Can not upload a json file with image mime type', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/data/redirects.json'); + const fileContents = await fs.readFile(originalFilePath); + await uploadImageRequest({fileContents, filename: 'redirects.json', contentType: 'image/gif'}) + .expectStatus(415) + .matchBodySnapshot({ + errors: [{ + id: anyErrorId + }] + }); + }); + + it('Can not upload a json file with image file extension', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/data/redirects.json'); + const fileContents = await fs.readFile(originalFilePath); + await uploadImageRequest({fileContents, filename: 'redirects.png', contentType: 'application/json'}) + .expectStatus(415) + .matchBodySnapshot({ + errors: [{ + id: anyErrorId + }] + }); + }); + + 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'}); + }); + + it('Can upload image with number suffix', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); + await uploadImageCheck({path: originalFilePath, filename: 'a-2.png', contentType: 'image/png'}); + }); + + 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'}); + }); + + 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'}); + }); + + it('Can upload multiple images with the same name in parallel', async function () { + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); + const originalFilePath2 = p.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg'); + + // Delay the first original file upload by 400ms to force race condition + const store = storage.getStorage('images'); + const saveStub = sinon.stub(store, 'save'); + let calls = 0; + saveStub.callsFake(async function (file) { + if (file.name.includes('_o')) { + calls += 1; + if (calls === 1) { + await sleep(400); + } + } + return saveStub.wrappedMethod.call(this, ...arguments); + }); + const firstPromise = uploadImageCheck({path: originalFilePath, filename: 'a.png', contentType: 'image/png'}); + await sleep(10); + + await Promise.all([ + firstPromise, + uploadImageCheck({path: originalFilePath2, filename: 'a.png', contentType: 'image/png', expectedFileName: 'a-1.png'}) + ]); + }); + + it('Can upload around midnight of month change', async function () { + const clock = sinon.useFakeTimers({now: new Date(2022, 0, 31, 23, 59, 59), shouldAdvanceTime: true}); + assert.equal(new Date().getMonth(), 0); + + const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png'); + + // Delay the first original file upload by 400ms to force race condition + const store = storage.getStorage('images'); + const saveStub = sinon.stub(store, 'save'); + let calls = 0; + saveStub.callsFake(async function (file) { + if (file.name.includes('_o')) { + calls += 1; + if (calls === 1) { + clock.tick(5000); + assert.equal(new Date().getMonth(), 1); + } + } + return saveStub.wrappedMethod.call(this, ...arguments); + }); + await uploadImageCheck({path: originalFilePath, filename: 'a.png', contentType: 'image/png'}); + clock.restore(); }); }); diff --git a/ghost/core/test/e2e-api/admin/posts.test.js b/ghost/core/test/e2e-api/admin/posts.test.js index 25d118c0e9..efa72536aa 100644 --- a/ghost/core/test/e2e-api/admin/posts.test.js +++ b/ghost/core/test/e2e-api/admin/posts.test.js @@ -118,7 +118,11 @@ describe('Posts API', function () { }; await agent - .post('/posts/?formats=mobiledoc,lexical,html') + .post('/posts/?formats=mobiledoc,lexical,html', { + headers: { + 'content-type': 'application/json' + } + }) .body({posts: [post]}) .expectStatus(201) .matchBodySnapshot({ diff --git a/ghost/core/test/unit/server/web/api/middleware/normalize-image.test.js b/ghost/core/test/unit/server/web/api/middleware/normalize-image.test.js deleted file mode 100644 index 795fb9d624..0000000000 --- a/ghost/core/test/unit/server/web/api/middleware/normalize-image.test.js +++ /dev/null @@ -1,82 +0,0 @@ -const should = require('should'); -const sinon = require('sinon'); -const configUtils = require('../../../../../utils/configUtils'); -const imageTransform = require('@tryghost/image-transform'); -const logging = require('@tryghost/logging'); -const normalize = require('../../../../../../core/server/web/api/middleware/normalize-image'); - -describe('normalize', function () { - let res; - let req; - - beforeEach(function () { - req = { - file: { - name: 'test', - path: '/test/path', - ext: '.jpg' - } - }; - - sinon.stub(imageTransform, 'resizeFromPath'); - sinon.stub(logging, 'error'); - }); - - afterEach(async function () { - sinon.restore(); - await configUtils.restore(); - }); - - it('should do manipulation by default', function (done) { - imageTransform.resizeFromPath.resolves(); - - normalize(req, res, function () { - imageTransform.resizeFromPath.calledOnce.should.be.true(); - done(); - }); - }); - - it('should add files array to request object with original and resized files', function (done) { - imageTransform.resizeFromPath.resolves(); - - normalize(req, res, function () { - req.files.length.should.be.equal(2); - done(); - }); - }); - - it('should not do manipulation without resize flag set', function (done) { - configUtils.set({ - imageOptimization: { - resize: false - } - }); - - normalize(req, res, function () { - imageTransform.resizeFromPath.called.should.be.false(); - done(); - }); - }); - - it('should not create files array when resizing fails', function (done) { - imageTransform.resizeFromPath.rejects(); - - normalize(req, res, () => { - logging.error.calledOnce.should.be.true(); - req.file.should.not.be.equal(undefined); - should.not.exist(req.files); - done(); - }); - }); - - ['.svg', '.svgz'].forEach(function (extension) { - it(`should skip resizing when file extension is ${extension}`, function (done) { - req.file.ext = extension; - normalize(req, res, function () { - req.file.should.not.be.equal(undefined); - should.not.exist(req.files); - done(); - }); - }); - }); -}); diff --git a/ghost/core/test/utils/fixtures/images/ghost-logo.svg b/ghost/core/test/utils/fixtures/images/ghost-logo.svg new file mode 100644 index 0000000000..1b8b5f12fb --- /dev/null +++ b/ghost/core/test/utils/fixtures/images/ghost-logo.svg @@ -0,0 +1,56 @@ + + + + + + + + + + +