Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-04-08 02:52:39 -05:00
Code Issues Activity

🐛 Fixed pagination w/ letters duplicating content (#8796)

Browse source 
refs #8700

- if you used a url e.g. /page/2abc/ ghost would interpret the 2 as /page/2/
- these urls should have returned 404, but instead were responding correctly
- this effectively creates duplicate pages
- added a test, but needed a dirty hack to get it to work 😞
- TODO: update casper fixture and use it in channel tests!
This commit is contained in:
Hannah Wolfe 2017-08-02 12:25:41 +04:00 • committed by Katharina Irrgang
parent 665f9f3926
commit aa7f3dd5fd
2 changed files with 14 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
core/server/controllers/frontend/channels.js
View file

@ -45,7 +45,7 @@ rssRouter = function rssRouter(channelConfig) {
baseRoute = '/rss/';
router.get(baseRoute, stack);
router.get(utils.url.urlJoin(baseRoute, ':page/'), stack);
router.get(utils.url.urlJoin(baseRoute, ':page(\\d+)/'), stack);
router.get('/feed/', function redirectToRSS(req, res) {
return utils.redirect301(res, utils.url.urlJoin(utils.url.getSubdir(), req.baseUrl, baseRoute));
});
@ -64,7 +64,7 @@ channelRouter = function router() {
var channelsRouter = express.Router({mergeParams: true}),
baseRoute = '/',
pageRoute = utils.url.urlJoin('/', config.get('routeKeywords').page, ':page/');
pageRoute = utils.url.urlJoin('/', config.get('routeKeywords').page, ':page(\\d+)/');
_.each(channelConfig.list(), function (channel) {
var channelRouter = express.Router({mergeParams: true}),

26
core/test/functional/routes/channel_spec.js
View file

@ -78,15 +78,6 @@ describe('Channel Routes', function () {
});
});
// @TODO: use theme from fixtures and don't rely on content/themes/casper
it.skip('should have a second page', function (done) {
request.get('/page/2/')
.expect('Content-Type', /html/)
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(200)
.end(doEnd(done));
});
it('should not have a third page', function (done) {
request.get('/page/3/')
.expect('Cache-Control', testUtils.cacheRules.private)
@ -147,14 +138,13 @@ describe('Channel Routes', function () {
});
describe('Paged', function () {
// Add enough posts to trigger pages for both the index (5 pp) and rss (15 pp)
// insertPosts adds 11 published posts, 1 draft post, 1 published static page and one draft page
// we then insert with max 5 which ensures we have 16 published posts
// Add enough posts to trigger pages for both the index (25 pp) and rss (15 pp)
// @TODO: change this whole thing to use the casper theme from fixtures
before(function (done) {
testUtils.initData().then(function () {
return testUtils.fixtures.insertPostsAndTags();
}).then(function () {
return testUtils.fixtures.insertMorePosts(5);
return testUtils.fixtures.insertMorePosts(25);
}).then(function () {
done();
}).catch(done);
@ -171,7 +161,7 @@ describe('Channel Routes', function () {
});
// @TODO: use theme from fixtures and don't rely on content/themes/casper
it.skip('should respond with html', function (done) {
it('should respond with html', function (done) {
request.get('/page/2/')
.expect('Content-Type', /html/)
.expect('Cache-Control', testUtils.cacheRules.public)
@ -179,6 +169,14 @@ describe('Channel Routes', function () {
.end(doEnd(done));
});
it('should not allow chars after the page number', function (done) {
request.get('/page/2abc/')
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(404)
.expect(/Page not found/)
.end(doEnd(done));
});
it('should redirect page 1', function (done) {
request.get('/page/1/')
.expect('Location', '/')