From 74273dabb3a2971777b4dc143ded848de8a3c441 Mon Sep 17 00:00:00 2001 From: dotch Date: Thu, 2 Apr 2015 03:30:55 +0200 Subject: [PATCH 1/4] return a 404 for not found api, module and lib routes --- config/lib/express.js | 7 ------- modules/core/server/routes/core.server.routes.js | 4 +++- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/config/lib/express.js b/config/lib/express.js index 2ad68145b2..c3f6b953d2 100644 --- a/config/lib/express.js +++ b/config/lib/express.js @@ -177,7 +177,6 @@ module.exports.initModulesServerRoutes = function (app) { * Configure error handling */ module.exports.initErrorRoutes = function (app) { - // Assume 'not found' in the error msgs is a 404. this is somewhat silly, but valid, you can do whatever you like, set properties, use instanceof etc. app.use(function (err, req, res, next) { // If the error object doesn't exists if (!err) return next(); @@ -188,12 +187,6 @@ module.exports.initErrorRoutes = function (app) { // Redirect to error page res.redirect('/server-error'); }); - - // Assume 404 since no middleware responded - app.use(function (req, res) { - // Redirect to not found page - res.redirect('/not-found'); - }); }; /** diff --git a/modules/core/server/routes/core.server.routes.js b/modules/core/server/routes/core.server.routes.js index 33a436641c..d58861fc91 100644 --- a/modules/core/server/routes/core.server.routes.js +++ b/modules/core/server/routes/core.server.routes.js @@ -6,7 +6,9 @@ module.exports = function(app) { // Define error pages app.route('/server-error').get(core.renderServerError); - app.route('/not-found').get(core.renderNotFound); + + // Return a 404 for all undefined api, module or lib routes + app.route('/:url(api|modules|lib)/*').get(core.renderNotFound); // Define application route app.route('/*').get(core.renderIndex); From ba1a4475e9613f2f9c34c98214fcf6fc32bbb159 Mon Sep 17 00:00:00 2001 From: Mikael Korpela Date: Mon, 18 May 2015 17:38:30 +0300 Subject: [PATCH 2/4] #501 Handle 404 errors at Express backend at at Angular frontend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `/{api|modules|lib}/*` returns error page when path doesn’t exist (from Express). - `/*` always returns index (from Express), but if `$state` doesn’t exist, Angular redirects to `/not-found` (no 404 status in that case though!) - If `Accept: application/json` header is present without `Accept: text/html`, return error as json. Hence looking at non existing /api/* paths with browser would show html error, but querying them with script would return json. - Slightly prettier 404 error Test: ```bash curl http://localhost:3000/api/notfound -4 -H "Accept: application/json" ``` => json error. ```bash curl http://localhost:3000/api/notfound -4 -H "Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0 .8" ``` => html error (imitates Chrome’s Accept header). Starting point was @dotch’s PL: https://github.com/meanjs/mean/pull/503 And `req.accepts()` idea came from http://stackoverflow.com/a/9802006 --- .../core/client/config/core.client.routes.js | 17 ++++++++----- .../core/client/views/404.client.view.html | 6 +++++ .../controllers/core.server.controller.js | 25 +++++++++++++++---- .../core/server/views/404.server.view.html | 6 +++-- 4 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 modules/core/client/views/404.client.view.html diff --git a/modules/core/client/config/core.client.routes.js b/modules/core/client/config/core.client.routes.js index 7328d0d213..fe5692bd35 100644 --- a/modules/core/client/config/core.client.routes.js +++ b/modules/core/client/config/core.client.routes.js @@ -3,14 +3,19 @@ // Setting up route angular.module('core').config(['$stateProvider', '$urlRouterProvider', function($stateProvider, $urlRouterProvider) { - // Redirect to home view when route not found - $urlRouterProvider.otherwise('/'); + + // Redirect to 404 when route not found + $urlRouterProvider.otherwise('not-found'); // Home state routing $stateProvider. - state('home', { - url: '/', - templateUrl: 'modules/core/views/home.client.view.html' - }); + state('home', { + url: '/', + templateUrl: 'modules/core/views/home.client.view.html' + }). + state('not-found', { + url: '/not-found', + templateUrl: 'modules/core/views/404.client.view.html' + }); } ]); diff --git a/modules/core/client/views/404.client.view.html b/modules/core/client/views/404.client.view.html new file mode 100644 index 0000000000..6e79d29f44 --- /dev/null +++ b/modules/core/client/views/404.client.view.html @@ -0,0 +1,6 @@ +

Page Not Found

+ diff --git a/modules/core/server/controllers/core.server.controller.js b/modules/core/server/controllers/core.server.controller.js index 3b427c38f7..efda2451db 100644 --- a/modules/core/server/controllers/core.server.controller.js +++ b/modules/core/server/controllers/core.server.controller.js @@ -1,7 +1,7 @@ 'use strict'; /** - * Render the main applicaion page + * Render the main application page */ exports.renderIndex = function(req, res) { res.render('modules/core/server/views/index', { @@ -19,10 +19,25 @@ exports.renderServerError = function(req, res) { }; /** - * Render the server not found page + * Render the server not found responses */ exports.renderNotFound = function(req, res) { - res.status(404).render('modules/core/server/views/404', { - url: req.originalUrl - }); + res.status(404); + + // Respond with html page + if (req.accepts('html')) { + res.render('modules/core/server/views/404', { + url: req.originalUrl + }); + return; + } + + // Respond with json to API calls + if (req.accepts('json')) { + res.json({ error: 'Path not found' }); + return; + } + + // Default to plain-text + res.type('txt').send('Path not found'); }; diff --git a/modules/core/server/views/404.server.view.html b/modules/core/server/views/404.server.view.html index 404076174c..94e371926a 100644 --- a/modules/core/server/views/404.server.view.html +++ b/modules/core/server/views/404.server.view.html @@ -2,7 +2,9 @@ {% block content %}

Page Not Found

-
+
+ {% endblock %} From fd170261ec9699c99bbceb3a5c5db1e0fab7da48 Mon Sep 17 00:00:00 2001 From: Mikael Korpela Date: Mon, 18 May 2015 19:22:56 +0300 Subject: [PATCH 3/4] #501 Use req.format() to content-negotiate correct response --- .../controllers/core.server.controller.js | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/modules/core/server/controllers/core.server.controller.js b/modules/core/server/controllers/core.server.controller.js index efda2451db..5fe58e65df 100644 --- a/modules/core/server/controllers/core.server.controller.js +++ b/modules/core/server/controllers/core.server.controller.js @@ -20,24 +20,23 @@ exports.renderServerError = function(req, res) { /** * Render the server not found responses + * Performs content-negotiation on the Accept HTTP header */ exports.renderNotFound = function(req, res) { - res.status(404); - // Respond with html page - if (req.accepts('html')) { - res.render('modules/core/server/views/404', { - url: req.originalUrl + res + .status(404) + .format({ + 'text/html': function(){ + res.render('modules/core/server/views/404', { + url: req.originalUrl + }); + }, + 'application/json': function(){ + res.json({ error: 'Path not found' }); + }, + 'default': function(){ + res.send('Path not found'); + } }); - return; - } - - // Respond with json to API calls - if (req.accepts('json')) { - res.json({ error: 'Path not found' }); - return; - } - - // Default to plain-text - res.type('txt').send('Path not found'); }; From 7070796c53affb5372d151c2c7699eef28137c72 Mon Sep 17 00:00:00 2001 From: Mikael Korpela Date: Mon, 18 May 2015 19:25:02 +0300 Subject: [PATCH 4/4] Prettier res.status().format() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (due tabs — my editor has tab-spacing set to 2 so I don’t notice when stuff like this looks crappy) --- modules/core/server/controllers/core.server.controller.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/modules/core/server/controllers/core.server.controller.js b/modules/core/server/controllers/core.server.controller.js index 5fe58e65df..45d5d76468 100644 --- a/modules/core/server/controllers/core.server.controller.js +++ b/modules/core/server/controllers/core.server.controller.js @@ -24,9 +24,7 @@ exports.renderServerError = function(req, res) { */ exports.renderNotFound = function(req, res) { - res - .status(404) - .format({ + res.status(404).format({ 'text/html': function(){ res.render('modules/core/server/views/404', { url: req.originalUrl