🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
const {agentProvider, fixtureManager, matchers} = require('../../utils/e2e-framework');
|
|
|
|
const FormData = require('form-data');
|
|
|
|
const p = require('path');
|
2023-03-10 14:53:35 +03:00
|
|
|
const {promises: fs} = require('fs');
|
2023-06-21 11:56:59 +03:00
|
|
|
const assert = require('assert/strict');
|
2020-05-27 20:47:53 +03:00
|
|
|
const config = require('../../../core/shared/config');
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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 {anyErrorId} = matchers;
|
|
|
|
const {imageSize} = require('../../../core/server/lib/image');
|
|
|
|
const configUtils = require('../../utils/configUtils');
|
2023-03-03 20:58:19 +03:00
|
|
|
const logging = require('@tryghost/logging');
|
2018-10-12 23:41:39 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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}`));
|
2023-06-21 11:56:59 +03:00
|
|
|
assert.equal(body.images[0].ref, ref === undefined ? null : ref);
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
|
|
|
|
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}
|
|
|
|
});
|
2018-10-12 23:41:39 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
// 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 () {
|
2020-11-30 17:25:22 +03:00
|
|
|
before(async function () {
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
const agents = await agentProvider.getAgentsWithFrontend();
|
|
|
|
agent = agents.adminAgent;
|
|
|
|
frontendAgent = agents.frontendAgent;
|
|
|
|
ghostServer = agents.ghostServer;
|
|
|
|
await fixtureManager.init();
|
|
|
|
await agent.loginAsOwner();
|
2018-10-12 23:41:39 +03:00
|
|
|
});
|
|
|
|
|
|
|
|
after(function () {
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
configUtils.restore();
|
|
|
|
ghostServer.stop();
|
2018-10-12 23:41:39 +03:00
|
|
|
});
|
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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();
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
it('Can upload a png', async function () {
|
|
|
|
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'});
|
2018-10-12 23:41:39 +03:00
|
|
|
});
|
|
|
|
|
2020-11-30 17:25:22 +03:00
|
|
|
it('Can upload a jpg', async function () {
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghosticon.jpg');
|
|
|
|
await uploadImageCheck({path: originalFilePath, filename: 'ghosticon.jpg', contentType: 'image/jpeg'});
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
it('Can upload a gif', async function () {
|
|
|
|
const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/loadingcat.gif');
|
|
|
|
await uploadImageCheck({path: originalFilePath, filename: 'loadingcat.gif', contentType: 'image/gif'});
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
it('Can upload a webp', async function () {
|
|
|
|
const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghosticon.webp');
|
|
|
|
await uploadImageCheck({path: originalFilePath, filename: 'ghosticon.webp', contentType: 'image/webp'});
|
2019-02-04 17:16:24 +03:00
|
|
|
});
|
2018-10-12 23:41:39 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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});
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
it('Can upload a square profile image', async function () {
|
|
|
|
const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/loadingcat_square.gif');
|
|
|
|
await uploadImageCheck({path: originalFilePath, filename: 'loadingcat_square.gif', contentType: 'image/gif'});
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
2024-03-11 23:50:55 +03:00
|
|
|
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);
|
|
|
|
});
|
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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);
|
2023-03-03 20:58:19 +03:00
|
|
|
const loggingStub = sinon.stub(logging, 'error');
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
await uploadImageRequest({fileContents, filename: 'redirects.json', contentType: 'application/json'})
|
|
|
|
.expectStatus(415)
|
|
|
|
.matchBodySnapshot({
|
|
|
|
errors: [{
|
|
|
|
id: anyErrorId
|
|
|
|
}]
|
|
|
|
});
|
2023-03-03 20:58:19 +03:00
|
|
|
sinon.assert.calledOnce(loggingStub);
|
2019-02-04 17:16:24 +03:00
|
|
|
});
|
2018-10-12 23:41:39 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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);
|
2023-03-03 20:58:19 +03:00
|
|
|
const loggingStub = sinon.stub(logging, 'error');
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
await uploadImageRequest({fileContents, filename: 'redirects', contentType: 'image/png'})
|
|
|
|
.expectStatus(415)
|
|
|
|
.matchBodySnapshot({
|
|
|
|
errors: [{
|
|
|
|
id: anyErrorId
|
|
|
|
}]
|
|
|
|
});
|
2023-03-03 20:58:19 +03:00
|
|
|
sinon.assert.calledOnce(loggingStub);
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
});
|
2021-07-02 11:37:54 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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);
|
2023-03-03 20:58:19 +03:00
|
|
|
const loggingStub = sinon.stub(logging, 'error');
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
await uploadImageRequest({fileContents, filename: 'redirects.json', contentType: 'image/gif'})
|
|
|
|
.expectStatus(415)
|
|
|
|
.matchBodySnapshot({
|
|
|
|
errors: [{
|
|
|
|
id: anyErrorId
|
|
|
|
}]
|
|
|
|
});
|
2023-03-03 20:58:19 +03:00
|
|
|
sinon.assert.calledOnce(loggingStub);
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
});
|
2021-07-02 11:37:54 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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);
|
2023-03-03 20:58:19 +03:00
|
|
|
const loggingStub = sinon.stub(logging, 'error');
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
await uploadImageRequest({fileContents, filename: 'redirects.png', contentType: 'application/json'})
|
|
|
|
.expectStatus(415)
|
|
|
|
.matchBodySnapshot({
|
|
|
|
errors: [{
|
|
|
|
id: anyErrorId
|
|
|
|
}]
|
|
|
|
});
|
2023-03-03 20:58:19 +03:00
|
|
|
sinon.assert.calledOnce(loggingStub);
|
2021-07-02 11:37:54 +03:00
|
|
|
});
|
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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'});
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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'});
|
|
|
|
});
|
2019-02-04 17:16:24 +03:00
|
|
|
|
🐛 Fixed storing original files for images (#16117)
fixes https://github.com/TryGhost/Team/issues/481
This change fixes an issue when multiple images with the same name are
uploaded in parallel. The current system does not guarantee that the
original filename is stored under NAME+`_o`, because the upload for the
original file and the resized file are happening in parallel.
Solution:
- Wait for the storage of the resized image (= the image without the _o
suffix) before storing the original file.
- When that is stored, use the generated file name of the stored image
to generate the filename with the _o suffix. This way, it will always
match and we don't risk both files to have a different number suffix.
We'll also set the `targetDir` argument when saving the file, to avoid
storing the original file in a different directory (when uploading a
file around midnight both files could be stored in 2023/01 and 2023/02).
Some extra optimisations needed with this fix:
- Previously when uploading image.jpg, while it already exists, it would
store two filenames on e.g., `image-3.jpg` and `image_o-3.jpg`. Note the
weird positioning of `_o`. This probably caused bugs when uploading
files named `image-3.jpg`, which would store the original in
`image-3_o.jpg`, but this original would never be used by the
handle-image-sizes middleware (it would look for `image_o-3.jpg`). This
fix would solve this weird naming issue, and make it more consistent.
But we need to make sure our middlewares (including handle-image-sizes)
will be able to handle both file locations to remain compatible with the
old format. This isn't additional work, because it would fix the old bug
too.
- Prevent uploading files that end with `_o`, e.g. by automatically
stripping that suffix from uploaded files. To prevent collisions.
Advantage(s):
- We keep the original file name, which is better for SEO.
- No changes required to the storage adapters.
Downside(s):
- The storage of both files will nog happen parallel any longer. But I
expect the performance implications to be minimal.
- Changes to the routing: normalize middleware is removed
2023-01-30 18:40:50 +03:00
|
|
|
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 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();
|
2018-10-12 23:41:39 +03:00
|
|
|
});
|
2024-03-12 17:21:57 +03:00
|
|
|
|
|
|
|
it('Does not return HTTP 500 when image processing fails', async function () {
|
|
|
|
sinon.stub(imageTransform, 'resizeFromPath').rejects(new Error('Image processing failed'));
|
|
|
|
|
|
|
|
const originalFilePath = p.join(__dirname, '/../../utils/fixtures/images/ghost-logo.png');
|
|
|
|
const fileContents = await fs.readFile(originalFilePath);
|
|
|
|
|
2024-03-12 17:56:40 +03:00
|
|
|
const loggingStub = sinon.stub(logging, 'error');
|
2024-03-12 17:21:57 +03:00
|
|
|
await uploadImageRequest({fileContents, filename: 'test.png', contentType: 'image/png'})
|
2024-03-12 17:56:40 +03:00
|
|
|
.expectStatus(400)
|
|
|
|
.matchBodySnapshot({
|
|
|
|
errors: [{
|
|
|
|
id: anyErrorId
|
|
|
|
}]
|
|
|
|
});
|
|
|
|
sinon.assert.calledOnce(loggingStub);
|
2024-03-12 17:21:57 +03:00
|
|
|
});
|
2018-10-12 23:41:39 +03:00
|
|
|
});
|