From 8c355349b3cd744c43129beb06a1568e340217e4 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 10 Dec 2015 15:00:02 +0000 Subject: [PATCH] No more soft 404s in pagination closes #6201 - redirects for page/1/ or rss/1/ are now 301s - any other invalid page now 404s --- core/server/controllers/frontend/index.js | 26 +- core/server/data/xml/rss/index.js | 10 +- core/server/errors/index.js | 4 +- core/server/routes/frontend.js | 49 ++- core/test/functional/routes/admin_spec.js | 8 +- core/test/functional/routes/frontend_spec.js | 127 ++++--- .../unit/controllers/frontend/index_spec.js | 331 ------------------ core/test/unit/rss_spec.js | 87 +---- 8 files changed, 129 insertions(+), 513 deletions(-) diff --git a/core/server/controllers/frontend/index.js b/core/server/controllers/frontend/index.js index 39e15894e1..1d90cc792c 100644 --- a/core/server/controllers/frontend/index.js +++ b/core/server/controllers/frontend/index.js @@ -45,7 +45,7 @@ function renderChannel(name) { return function renderChannel(req, res, next) { // Parse the parameters we need from the URL var channelOpts = channelConfig(name), - pageParam = req.params.page !== undefined ? parseInt(req.params.page, 10) : 1, + pageParam = req.params.page !== undefined ? req.params.page : 1, slugParam = req.params.slug ? safeString(req.params.slug) : undefined; // Ensure we at least have an empty object for postOptions @@ -54,31 +54,11 @@ function renderChannel(name) { channelOpts.postOptions.page = pageParam; channelOpts.slugParam = slugParam; - // @TODO this should really use the url building code in config.url - function createUrl(page) { - var url = config.paths.subdir + channelOpts.route; - - if (slugParam) { - url = url.replace(':slug', slugParam); - } - - if (page && page > 1) { - url += 'page/' + page + '/'; - } - - return url; - } - - // If the page parameter isn't something sensible, redirect - if (isNaN(pageParam) || pageParam < 1 || (req.params.page !== undefined && pageParam === 1)) { - return res.redirect(createUrl()); - } - // Call fetchData to get everything we need from the API return fetchData(channelOpts).then(function handleResult(result) { - // If page is greater than number of pages we have, redirect to last page + // If page is greater than number of pages we have, go straight to 404 if (pageParam > result.meta.pagination.pages) { - return res.redirect(createUrl(result.meta.pagination.pages)); + return next(new errors.NotFoundError()); } // @TODO: figure out if this can be removed, it's supposed to ensure that absolutely URLs get generated diff --git a/core/server/data/xml/rss/index.js b/core/server/data/xml/rss/index.js index 6e5cda4ec1..9dcc53bfc7 100644 --- a/core/server/data/xml/rss/index.js +++ b/core/server/data/xml/rss/index.js @@ -5,6 +5,7 @@ var _ = require('lodash'), RSS = require('rss'), url = require('url'), config = require('../../../config'), + errors = require('../../../errors'), filters = require('../../../filters'), // Really ugly temporary hack for location of things @@ -190,7 +191,7 @@ generateFeed = function generateFeed(data) { generate = function generate(req, res, next) { // Initialize RSS - var pageParam = req.params.page !== undefined ? parseInt(req.params.page, 10) : 1, + var pageParam = req.params.page !== undefined ? req.params.page : 1, slugParam = req.params.slug, baseUrl = getBaseUrl(req, slugParam); @@ -201,17 +202,12 @@ generate = function generate(req, res, next) { req.channelConfig.slugParam = slugParam; - // No negative pages, or page 1 - if (isNaN(pageParam) || pageParam < 1 || (req.params.page !== undefined && pageParam === 1)) { - return res.redirect(baseUrl); - } - return getData(req.channelConfig).then(function then(data) { var maxPage = data.results.meta.pagination.pages; // If page is greater than number of pages we have, redirect to last page if (pageParam > maxPage) { - return res.redirect(baseUrl + maxPage + '/'); + return next(new errors.NotFoundError()); } data.version = res.locals.safeVersion; diff --git a/core/server/errors/index.js b/core/server/errors/index.js index 6c010ab8e3..3068ecf2de 100644 --- a/core/server/errors/index.js +++ b/core/server/errors/index.js @@ -280,7 +280,7 @@ errors = { function renderErrorInt(errorView) { var stack = null; - if (process.env.NODE_ENV !== 'production' && err.stack) { + if (code !== 404 && process.env.NODE_ENV !== 'production' && err.stack) { stack = parseStack(err.stack); } @@ -336,7 +336,7 @@ errors = { // 500 errors should never be cached res.set({'Cache-Control': 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'}); - if (err.status === 404) { + if (err.status === 404 || err.code === 404) { return this.error404(req, res, next); } diff --git a/core/server/routes/frontend.js b/core/server/routes/frontend.js index cb93ff5fc1..352321cd43 100644 --- a/core/server/routes/frontend.js +++ b/core/server/routes/frontend.js @@ -1,5 +1,6 @@ var frontend = require('../controllers/frontend'), config = require('../config'), + errors = require('../errors'), express = require('express'), utils = require('../utils'), @@ -15,22 +16,46 @@ frontendRoutes = function frontendRoutes(middleware) { rssRouter = express.Router({mergeParams: true}), privateRouter = express.Router(); + function redirect301(res, path) { + /*jslint unparam:true*/ + res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); + res.redirect(301, path); + } + + function handlePageParam(req, res, next, page) { + var pageRegex = new RegExp('/' + routeKeywords.page + '/(.*)?/'), + rssRegex = new RegExp('/rss/(.*)?/'); + + page = parseInt(page, 10); + + if (page === 1) { + // Page 1 is an alias, do a permanent 301 redirect + if (rssRegex.test(req.url)) { + return redirect301(res, req.originalUrl.replace(rssRegex, '/rss/')); + } else { + return redirect301(res, req.originalUrl.replace(pageRegex, '/')); + } + } else if (page < 1 || isNaN(page)) { + // Nothing less than 1 is a valid page number, go straight to a 404 + return next(new errors.NotFoundError()); + } else { + // Set req.params.page to the already parsed number, and continue + req.params.page = page; + return next(); + } + } + // ### Admin routes router.get(/^\/(logout|signout)\/$/, function redirectToSignout(req, res) { - /*jslint unparam:true*/ - res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); - res.redirect(301, subdir + '/ghost/signout/'); + redirect301(res, subdir + '/ghost/signout/'); }); router.get(/^\/signup\/$/, function redirectToSignup(req, res) { - /*jslint unparam:true*/ - res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); - res.redirect(301, subdir + '/ghost/signup/'); + redirect301(res, subdir + '/ghost/signup/'); }); // redirect to /ghost and let that do the authentication to prevent redirects to /ghost//admin etc. router.get(/^\/((ghost-admin|admin|wp-admin|dashboard|signin|login)\/?)$/, function redirectToAdmin(req, res) { - /*jslint unparam:true*/ - res.redirect(subdir + '/ghost/'); + redirect301(res, subdir + '/ghost/'); }); // password-protected frontend route @@ -49,14 +74,14 @@ frontendRoutes = function frontendRoutes(middleware) { rssRouter.route('/rss/').get(frontend.rss); rssRouter.route('/rss/:page/').get(frontend.rss); rssRouter.route('/feed/').get(function redirect(req, res) { - /*jshint unused:true*/ - res.set({'Cache-Control': 'public, max-age=' + utils.ONE_YEAR_S}); - res.redirect(301, subdir + '/rss/'); + redirect301(res, subdir + '/rss/'); }); + rssRouter.param('page', handlePageParam); // Index indexRouter.route('/').get(frontend.index); indexRouter.route('/' + routeKeywords.page + '/:page/').get(frontend.index); + indexRouter.param('page', handlePageParam); indexRouter.use(rssRouter); // Tags @@ -65,6 +90,7 @@ frontendRoutes = function frontendRoutes(middleware) { tagRouter.route('/edit?').get(function redirect(req, res) { res.redirect(subdir + '/ghost/settings/tags/' + req.params.slug + '/'); }); + tagRouter.param('page', handlePageParam); tagRouter.use(rssRouter); // Authors @@ -73,6 +99,7 @@ frontendRoutes = function frontendRoutes(middleware) { res.redirect(subdir + '/ghost/team/' + req.params.slug + '/'); }); authorRouter.route('/' + routeKeywords.page + '/:page/').get(frontend.author); + authorRouter.param('page', handlePageParam); authorRouter.use(rssRouter); // Mount the Routers diff --git a/core/test/functional/routes/admin_spec.js b/core/test/functional/routes/admin_spec.js index 4ceb92fe41..fd0d147d95 100644 --- a/core/test/functional/routes/admin_spec.js +++ b/core/test/functional/routes/admin_spec.js @@ -85,16 +85,16 @@ describe('Admin Routing', function () { it('should redirect /signin/ to /ghost/', function (done) { request.get('/signin/') .expect('Location', '/ghost/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEndNoAuth(done)); }); it('should redirect /admin/ to /ghost/', function (done) { request.get('/admin/') .expect('Location', '/ghost/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEndNoAuth(done)); }); diff --git a/core/test/functional/routes/frontend_spec.js b/core/test/functional/routes/frontend_spec.js index ec9ca19aff..c0d9256cc3 100644 --- a/core/test/functional/routes/frontend_spec.js +++ b/core/test/functional/routes/frontend_spec.js @@ -151,9 +151,9 @@ describe('Frontend Routing', function () { it('should not have as second page', function (done) { request.get('/page/2/') - .expect('Location', '/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); }); @@ -407,25 +407,40 @@ describe('Frontend Routing', function () { it('should redirect page 1', function (done) { request.get('/page/1/') .expect('Location', '/') - .expect('Cache-Control', testUtils.cacheRules.public) - // TODO: This should probably be a 301? - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEnd(done)); }); - it('should redirect to last page if page too high', function (done) { + it('should 404 if page too high', function (done) { request.get('/page/4/') - .expect('Location', '/page/3/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); - it('should redirect to first page if page too low', function (done) { + it('should 404 if page is zero', function (done) { request.get('/page/0/') - .expect('Location', '/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) + .end(doEnd(done)); + }); + + it('should 404 if page is less than zero', function (done) { + request.get('/page/-5/') + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) + .end(doEnd(done)); + }); + + it('should 404 if page is NaN', function (done) { + request.get('/page/one/') + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); }); @@ -555,25 +570,24 @@ describe('Frontend Routing', function () { it('should redirect page 1', function (done) { request.get('/author/ghost-owner/page/1/') .expect('Location', '/author/ghost-owner/') - .expect('Cache-Control', testUtils.cacheRules.public) - // TODO: This should probably be a 301? - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEnd(done)); }); - it('should redirect to last page if page too high', function (done) { + it('should 404 if page too high', function (done) { request.get('/author/ghost-owner/page/4/') - .expect('Location', '/author/ghost-owner/page/3/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); - it('should redirect to first page if page too low', function (done) { + it('should 404 if page too low', function (done) { request.get('/author/ghost-owner/page/0/') - .expect('Location', '/author/ghost-owner/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); @@ -597,25 +611,24 @@ describe('Frontend Routing', function () { it('should redirect page 1', function (done) { request.get('/author/ghost-owner/rss/1/') .expect('Location', '/author/ghost-owner/rss/') - .expect('Cache-Control', testUtils.cacheRules.public) - // TODO: This should probably be a 301? - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEnd(done)); }); - it('should redirect to last page if page too high', function (done) { + it('should 404 if page too high', function (done) { request.get('/author/ghost-owner/rss/2/') - .expect('Location', '/author/ghost-owner/rss/1/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); - it('should redirect to first page if page too low', function (done) { + it('should 404 if page too low', function (done) { request.get('/author/ghost-owner/rss/0/') - .expect('Location', '/author/ghost-owner/rss/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); }); @@ -690,25 +703,24 @@ describe('Frontend Routing', function () { it('should redirect page 1', function (done) { request.get('/tag/injection/page/1/') .expect('Location', '/tag/injection/') - .expect('Cache-Control', testUtils.cacheRules.public) - // TODO: This should probably be a 301? - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEnd(done)); }); - it('should redirect to last page if page too high', function (done) { + it('should 404 if page too high', function (done) { request.get('/tag/injection/page/4/') - .expect('Location', '/tag/injection/page/3/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); - it('should redirect to first page if page too low', function (done) { + it('should 404 if page too low', function (done) { request.get('/tag/injection/page/0/') - .expect('Location', '/tag/injection/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); @@ -732,25 +744,24 @@ describe('Frontend Routing', function () { it('should redirect page 1', function (done) { request.get('/tag/getting-started/rss/1/') .expect('Location', '/tag/getting-started/rss/') - .expect('Cache-Control', testUtils.cacheRules.public) - // TODO: This should probably be a 301? - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.year) + .expect(301) .end(doEnd(done)); }); - it('should redirect to last page if page too high', function (done) { + it('should 404 if page too high', function (done) { request.get('/tag/getting-started/rss/2/') - .expect('Location', '/tag/getting-started/rss/1/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); - it('should redirect to first page if page too low', function (done) { + it('should 404 if page too low', function (done) { request.get('/tag/getting-started/rss/0/') - .expect('Location', '/tag/getting-started/rss/') - .expect('Cache-Control', testUtils.cacheRules.public) - .expect(302) + .expect('Cache-Control', testUtils.cacheRules.private) + .expect(404) + .expect(/Page not found/) .end(doEnd(done)); }); }); diff --git a/core/test/unit/controllers/frontend/index_spec.js b/core/test/unit/controllers/frontend/index_spec.js index b52fa39667..c1c690bc15 100644 --- a/core/test/unit/controllers/frontend/index_spec.js +++ b/core/test/unit/controllers/frontend/index_spec.js @@ -37,119 +37,6 @@ describe('Frontend Controller', function () { }; } - describe('index redirects', function () { - var res, - req; - - beforeEach(function () { - res = { - redirect: sandbox.spy(), - render: sandbox.spy() - }; - - req = { - params: {page: 0}, - route: {path: '/page/:page/'} - }; - - sandbox.stub(api.posts, 'browse', function () { - return Promise.resolve({posts: {}, meta: {pagination: {pages: 3}}}); - }); - - apiSettingsStub = sandbox.stub(api.settings, 'read'); - apiSettingsStub.withArgs('postsPerPage').returns(Promise.resolve({ - settings: [{ - key: 'postsPerPage', - value: 5 - }] - })); - }); - - it('Redirects to home if page number is -1', function () { - req.params.page = -1; - - frontend.index(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 0', function () { - req.params.page = 0; - - frontend.index(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 1', function () { - req.params.page = 1; - - frontend.index(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 0 with subdirectory', function () { - config.set({ - url: 'http://my-ghost-blog.com/blog' - }); - - req.params.page = 0; - - frontend.index(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 1 with subdirectory', function () { - config.set({ - url: 'http://my-ghost-blog.com/blog' - }); - - req.params.page = 1; - - frontend.index(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to last page if page number too big', function (done) { - req.params.page = 4; - - frontend.index(req, res, done).then(function () { - res.redirect.called.should.be.true; - res.redirect.calledWith('/page/3/').should.be.true; - res.render.called.should.be.false; - done(); - }).catch(done); - }); - - it('Redirects to last page if page number too big with subdirectory', function (done) { - config.set({ - url: 'http://my-ghost-blog.com/blog' - }); - - req.params.page = 4; - - frontend.index(req, res, done).then(function () { - res.redirect.calledOnce.should.be.true; - res.redirect.calledWith('/blog/page/3/').should.be.true; - res.render.called.should.be.false; - done(); - }).catch(done); - }); - }); - describe('index', function () { var req, res; @@ -330,110 +217,6 @@ describe('Frontend Controller', function () { }); }); - describe('tag redirects', function () { - var res, req; - - beforeEach(function () { - res = { - redirect: sandbox.spy(), - render: sandbox.spy() - }; - - req = { - path: '/', params: {}, route: {} - }; - - sandbox.stub(api.posts, 'browse').returns(new Promise.resolve({posts: [{}], meta: {pagination: {pages: 3}}})); - sandbox.stub(api.tags, 'read').returns(new Promise.resolve({tags: [{}]})); - - apiSettingsStub = sandbox.stub(api.settings, 'read'); - apiSettingsStub.withArgs('postsPerPage').returns(Promise.resolve({ - settings: [{ - key: 'postsPerPage', - value: 5 - }] - })); - }); - - it('Redirects to base tag page if page number is -1', function () { - req.params = {page: -1, slug: 'pumpkin'}; - - frontend.tag(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/tag/pumpkin/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to base tag page if page number is 0', function () { - req.params = {page: 0, slug: 'pumpkin'}; - - frontend.tag(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/tag/pumpkin/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to base tag page if page number is 1', function () { - req.params = {page: 1, slug: 'pumpkin'}; - - frontend.tag(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/tag/pumpkin/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to base tag page if page number is 0 with subdirectory', function () { - config.set({url: 'http://my-ghost-blog.com/blog/'}); - - req.params = {page: 0, slug: 'pumpkin'}; - - frontend.tag(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/tag/pumpkin/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to base tag page if page number is 1 with subdirectory', function () { - config.set({url: 'http://my-ghost-blog.com/blog/'}); - - req.params = {page: 1, slug: 'pumpkin'}; - - frontend.tag(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/tag/pumpkin/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to last page if page number too big', function (done) { - req.params = {page: 4, slug: 'pumpkin'}; - - frontend.tag(req, res, done).then(function () { - res.redirect.called.should.be.true; - res.redirect.calledWith('/tag/pumpkin/page/3/').should.be.true; - res.render.called.should.be.false; - done(); - }).catch(done); - }); - - it('Redirects to last page if page number too big with subdirectory', function (done) { - config.set({url: 'http://my-ghost-blog.com/blog/'}); - - req.params = {page: 4, slug: 'pumpkin'}; - - frontend.tag(req, res, done).then(function () { - res.redirect.calledOnce.should.be.true; - res.redirect.calledWith('/blog/tag/pumpkin/page/3/').should.be.true; - res.render.called.should.be.false; - done(); - }).catch(done); - }); - }); - describe('single', function () { var req, res, casper, mockPosts = [{ posts: [{ @@ -1130,120 +913,6 @@ describe('Frontend Controller', function () { }); }); - describe('rss redirects', function () { - var res, - apiUsersStub; - - beforeEach(function () { - res = { - locals: {version: ''}, - redirect: sandbox.spy(), - render: sandbox.spy() - }; - - sandbox.stub(api.posts, 'browse', function () { - return Promise.resolve({posts: {}, meta: {pagination: {pages: 3}}}); - }); - - apiUsersStub = sandbox.stub(api.users, 'read').returns(Promise.resolve({})); - - apiSettingsStub = sandbox.stub(api.settings, 'read'); - apiSettingsStub.withArgs('permalinks').returns(Promise.resolve({ - settings: [{ - key: 'permalinks', - value: '/:slug/' - }] - })); - }); - - it('Redirects to rss if page number is 0', function () { - var req = {params: {page: -1}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to rss if page number is 0', function () { - var req = {params: {page: 0}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 1', function () { - var req = {params: {page: 1}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 0 with subdirectory', function () { - config.set({url: 'http://testurl.com/blog'}); - - var req = {params: {page: 0}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to home if page number is 1 with subdirectory', function () { - config.set({url: 'http://testurl.com/blog'}); - - var req = {params: {page: 1}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to last page if page number too big', function (done) { - config.set({url: 'http://testurl.com/'}); - - var req = {params: {page: 4}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, done).then(function () { - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/3/').should.be.true; - res.render.called.should.be.false; - done(); - }).catch(done); - }); - - it('Redirects to last page if page number too big with subdirectory', function (done) { - config.set({url: 'http://testurl.com/blog'}); - - var req = {params: {page: 4}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - - frontend.rss(req, res, done).then(function () { - res.redirect.calledOnce.should.be.true; - res.redirect.calledWith('/blog/rss/3/').should.be.true; - res.render.called.should.be.false; - done(); - }).catch(done); - }); - }); - describe('private', function () { var res, req, defaultPath; diff --git a/core/test/unit/rss_spec.js b/core/test/unit/rss_spec.js index d21f8a65d8..5a253a05ff 100644 --- a/core/test/unit/rss_spec.js +++ b/core/test/unit/rss_spec.js @@ -399,7 +399,7 @@ describe('RSS', function () { }); }); - describe('redirects', function () { + describe('pagination', function () { beforeEach(function () { res = { locals: {version: ''}, @@ -417,76 +417,7 @@ describe('RSS', function () { }); }); - it('Redirects to /rss/ if page number is -1', function () { - req = {params: {page: -1}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - req.channelConfig = channelConfig('index'); - req.channelConfig.isRSS = true; - - rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to /rss/ if page number is 0', function () { - req = {params: {page: 0}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - req.channelConfig = channelConfig('index'); - req.channelConfig.isRSS = true; - - rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to /rss/ if page number is 1', function () { - req = {params: {page: 1}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - req.channelConfig = channelConfig('index'); - req.channelConfig.isRSS = true; - - rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to /blog/rss/ if page number is 0 with subdirectory', function () { - config.set({url: 'http://testurl.com/blog'}); - - req = {params: {page: 0}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - req.channelConfig = channelConfig('index'); - req.channelConfig.isRSS = true; - - rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to /blog/rss/ if page number is 1 with subdirectory', function () { - config.set({url: 'http://testurl.com/blog'}); - - req = {params: {page: 1}, route: {path: '/rss/:page/'}}; - req.originalUrl = req.route.path.replace(':page', req.params.page); - req.channelConfig = channelConfig('index'); - req.channelConfig.isRSS = true; - - rss(req, res, null); - - res.redirect.called.should.be.true; - res.redirect.calledWith('/blog/rss/').should.be.true; - res.render.called.should.be.false; - }); - - it('Redirects to last page if page number too big', function (done) { + it('Should 404 if page number too big', function (done) { config.set({url: 'http://testurl.com/'}); req = {params: {page: 4}, route: {path: '/rss/:page/'}}; @@ -494,9 +425,10 @@ describe('RSS', function () { req.channelConfig = channelConfig('index'); req.channelConfig.isRSS = true; - rss(req, res, failTest(done)).then(function () { - res.redirect.called.should.be.true; - res.redirect.calledWith('/rss/3/').should.be.true; + rss(req, res, function (err) { + should.exist(err); + err.code.should.eql(404); + res.redirect.called.should.be.false; res.render.called.should.be.false; done(); }).catch(done); @@ -510,9 +442,10 @@ describe('RSS', function () { req.channelConfig = channelConfig('index'); req.channelConfig.isRSS = true; - rss(req, res, failTest(done)).then(function () { - res.redirect.calledOnce.should.be.true; - res.redirect.calledWith('/blog/rss/3/').should.be.true; + rss(req, res, function (err) { + should.exist(err); + err.code.should.eql(404); + res.redirect.called.should.be.false; res.render.called.should.be.false; done(); }).catch(done);