Skip to content

Commit 984bca4

Browse files
committed
🔥 Removed serving files w/o extension from themes
ref https://linear.app/ghost/issue/PROD-2214/return-404-from-frontends-assets-folder ref https://linear.app/ghost/issue/PROD-2322/improve-theme-static-asset-handling - If the path of a request doesn't have an extension, it's probably not a file - If there's no extension, skip the static theme middleware as that is for serving files - This change allows us to add fallthrough to theme assets, helping us to render 404s properly - However, this change also has the side effect of meaning any file that doesn't have an extension won't get served from the theme by Ghost anymore
1 parent b490682 commit 984bca4

File tree

3 files changed

+109
-4
lines changed

3 files changed

+109
-4
lines changed

ghost/core/core/frontend/web/middleware/static-theme.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ function forwardToExpressStatic(req, res, next) {
6969

7070
function staticTheme() {
7171
return function denyStatic(req, res, next) {
72+
if (!path.extname(req.path)) {
73+
return next();
74+
}
75+
7276
if (!isAllowedFile(req.path.toLowerCase()) && isDeniedFile(req.path.toLowerCase())) {
7377
return next();
7478
}

ghost/core/test/legacy/site/frontend.test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ describe('Frontend Routing', function () {
101101
});
102102

103103
it('Single post should sanitize double slashes when redirecting uppercase', function (done) {
104-
request.get('///Google.com/')
105-
.expect('Location', '/google.com/')
104+
request.get('///Google/')
105+
.expect('Location', '/google/')
106106
.expect('Cache-Control', testUtils.cacheRules.year)
107107
.expect(301)
108108
.end(doEnd(done));

ghost/core/test/unit/frontend/web/middleware/static-theme.test.js

Lines changed: 103 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,17 @@ describe('staticTheme', function () {
1313

1414
beforeEach(function () {
1515
req = {};
16-
res = {};
16+
res = {
17+
setHeader: sinon.stub()
18+
};
1719

1820
activeThemeStub = sinon.stub(themeEngine, 'getActive').returns({
1921
path: 'my/fake/path'
2022
});
2123

22-
expressStaticStub = sinon.spy(express, 'static');
24+
expressStaticStub = sinon.stub(express, 'static').returns(function (_req, _res, _next) {
25+
_next();
26+
});
2327
});
2428

2529
afterEach(function () {
@@ -225,4 +229,101 @@ describe('staticTheme', function () {
225229
done();
226230
});
227231
});
232+
233+
describe('paths without file extensions', function () {
234+
it('should skip for root path /', function (done) {
235+
req.path = '/';
236+
237+
staticTheme()(req, res, function next() {
238+
activeThemeStub.called.should.be.false();
239+
expressStaticStub.called.should.be.false();
240+
241+
done();
242+
});
243+
});
244+
245+
it('should skip for /about/', function (done) {
246+
req.path = '/about/';
247+
248+
staticTheme()(req, res, function next() {
249+
activeThemeStub.called.should.be.false();
250+
expressStaticStub.called.should.be.false();
251+
252+
done();
253+
});
254+
});
255+
256+
it('should skip for /blog/my-post/', function (done) {
257+
req.path = '/blog/my-post/';
258+
259+
staticTheme()(req, res, function next() {
260+
activeThemeStub.called.should.be.false();
261+
expressStaticStub.called.should.be.false();
262+
263+
done();
264+
});
265+
});
266+
267+
it('should skip for path without trailing slash /contact', function (done) {
268+
req.path = '/contact';
269+
270+
staticTheme()(req, res, function next() {
271+
activeThemeStub.called.should.be.false();
272+
expressStaticStub.called.should.be.false();
273+
274+
done();
275+
});
276+
});
277+
278+
it('should NOT skip for file with extension without trailing slash', function (done) {
279+
req.path = '/somefile.txt';
280+
281+
staticTheme()(req, res, function next() {
282+
// Specifically gets called twice
283+
activeThemeStub.calledTwice.should.be.true();
284+
expressStaticStub.called.should.be.true();
285+
286+
// Check that express static gets called with the theme path + maxAge
287+
should.exist(expressStaticStub.firstCall.args);
288+
expressStaticStub.firstCall.args[0].should.eql('my/fake/path');
289+
expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge');
290+
291+
done();
292+
});
293+
});
294+
295+
it('should NOT skip for file with extension with trailing slash', function (done) {
296+
req.path = '/somefile.txt/';
297+
298+
staticTheme()(req, res, function next() {
299+
// Specifically gets called twice
300+
activeThemeStub.calledTwice.should.be.true();
301+
expressStaticStub.called.should.be.true();
302+
303+
// Check that express static gets called with the theme path + maxAge
304+
should.exist(expressStaticStub.firstCall.args);
305+
expressStaticStub.firstCall.args[0].should.eql('my/fake/path');
306+
expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge');
307+
308+
done();
309+
});
310+
});
311+
312+
it('should NOT skip for deeply nested file with extension', function (done) {
313+
req.path = '/deep/nested/path/file.css';
314+
315+
staticTheme()(req, res, function next() {
316+
// Specifically gets called twice
317+
activeThemeStub.calledTwice.should.be.true();
318+
expressStaticStub.called.should.be.true();
319+
320+
// Check that express static gets called with the theme path + maxAge
321+
should.exist(expressStaticStub.firstCall.args);
322+
expressStaticStub.firstCall.args[0].should.eql('my/fake/path');
323+
expressStaticStub.firstCall.args[1].should.be.an.Object().with.property('maxAge');
324+
325+
done();
326+
});
327+
});
328+
});
228329
});

0 commit comments

Comments
 (0)