Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions ghost/core/core/server/adapters/storage/S3Storage.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import fs from 'node:fs';
import path from 'node:path';
import type {Request, Response, NextFunction, RequestHandler} from 'express';
import StorageBase from 'ghost-storage-base';
import tpl from '@tryghost/tpl';
import errors from '@tryghost/errors';
Expand Down Expand Up @@ -334,9 +335,16 @@ export default class S3Storage extends StorageBase {
}
}

serve() {
return function (_req: unknown, _res: unknown, next: (err?: unknown) => void) {
next();
serve(): RequestHandler {
return (req: Request, res: Response, next: NextFunction) => {
const relativePath = req.path.replace(/^\/+/, '');

if (!relativePath) {
return next();
}

const key = this.buildKey(relativePath);
res.redirect(301, `${this.cdnUrl}/${key}`);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing return statement on redirect? Not a bug here but adding return res.redirect(...) is a common pattern. Also makes it consistent with the early return return next();

};
}

Expand Down
123 changes: 118 additions & 5 deletions ghost/core/test/unit/server/adapters/storage/S3Storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,125 @@ describe('S3Storage', function () {
}, /missing expected storagePath/);
});

it('serve middleware short-circuits to next handler', function (done) {
const {storage} = createStorage();
describe('serve middleware', function () {
it('redirects to CDN URL with 301 status', function () {
const {storage} = createStorage();

const middleware = storage.serve();
const req = {path: '/2024/06/video.mp4'};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

sinon.assert.calledOnce(res.redirect);
sinon.assert.calledWith(
res.redirect,
301,
'https://cdn.example.com/configurable/prefix/content/files/2024/06/video.mp4'
);
sinon.assert.notCalled(next);
});

it('redirects without tenant prefix when not configured', function () {
const {storage} = createStorage({tenantPrefix: ''});

const middleware = storage.serve();
const req = {path: '/2024/06/podcast.mp3'};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

sinon.assert.calledOnce(res.redirect);
sinon.assert.calledWith(
res.redirect,
301,
'https://cdn.example.com/content/files/2024/06/podcast.mp3'
);
sinon.assert.notCalled(next);
});

it('handles deeply nested paths', function () {
const {storage} = createStorage();

const middleware = storage.serve();
const req = {path: '/2024/06/subfolder/another/video.mp4'};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

sinon.assert.calledOnce(res.redirect);
sinon.assert.calledWith(
res.redirect,
301,
'https://cdn.example.com/configurable/prefix/content/files/2024/06/subfolder/another/video.mp4'
);
});

it('preserves URL-encoded characters in path', function () {
const {storage} = createStorage({tenantPrefix: ''});

const middleware = storage.serve();
const req = {path: '/2024/06/my%20file%20(1).mp4'};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

const middleware = storage.serve();
middleware({} as any, {} as any, () => {
done();
sinon.assert.calledOnce(res.redirect);
sinon.assert.calledWith(
res.redirect,
301,
'https://cdn.example.com/content/files/2024/06/my%20file%20(1).mp4'
);
});

it('falls through to next middleware for empty path', function () {
const {storage} = createStorage();

const middleware = storage.serve();
const req = {path: ''};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

sinon.assert.notCalled(res.redirect);
sinon.assert.calledOnce(next);
});

it('falls through to next middleware for root path', function () {
const {storage} = createStorage();

const middleware = storage.serve();
const req = {path: '/'};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

sinon.assert.notCalled(res.redirect);
sinon.assert.calledOnce(next);
});

it('strips leading slashes from path when building redirect URL', function () {
const {storage} = createStorage({tenantPrefix: ''});

const middleware = storage.serve();
const req = {path: '///2024/06/file.mp4'};
const res = {redirect: sinon.stub()};
const next = sinon.stub();

middleware(req as any, res as any, next);

sinon.assert.calledOnce(res.redirect);
sinon.assert.calledWith(
res.redirect,
301,
'https://cdn.example.com/content/files/2024/06/file.mp4'
);
});
});

Expand Down
Loading