From adcff7689a536c9cda9c9f1b1a95c9ebefffd21f Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Wed, 5 Aug 2015 22:16:02 -0400 Subject: [PATCH 01/13] Fix dependencies and added root parameters to scripts --- package.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index 43be103bf..67a4379d1 100644 --- a/package.json +++ b/package.json @@ -38,17 +38,17 @@ "li": "^1.0.1", "mime": "^1.3.4", "n3": "^0.4.3", + "negotiator": "^0.5.3", "node-regexp": "^0.1.1", "node-uuid": "^1.4.3", "nomnom": "^1.8.1", "raw-body": "^2.1.2", - "rdflib": "^0.2.6", + "rdflib": "^0.2.8", "redis": "^0.12.1", "request": "^2.58.0", "response-time": "^2.3.1", "string": "^3.3.0", - "webid": "^0.2.2", - "negotiator": "^0.5.3" + "webid": "^0.2.2" }, "devDependencies": { "chai": "^3.0.0", @@ -63,9 +63,9 @@ "test-acl": "./node_modules/mocha/bin/mocha ./test/acl.js", "test-params": "./node_modules/mocha/bin/mocha ./test/params.js", "test-http": "./node_modules/mocha/bin/mocha ./test/http.js", - "test-formats": "./node_modules/mocha/bin/mocha ./test/http.js", - "ldp-webid": "node ./bin/ldnode.js --webid --cert ./test/keys/cert.pem --key ./test/keys/key.pem -v", - "ldp-ssl": "node ./bin/ldnode.js --cert ./test/keys/cert.pem --key ./test/keys/key.pem -v" + "test-formats": "./node_modules/mocha/bin/mocha ./test/formats.js", + "ldp-webid": "node ./bin/ldnode.js --webid --cert ./test/keys/cert.pem --key ./test/keys/key.pem -v -r ./test/resources", + "ldp-ssl": "node ./bin/ldnode.js --cert ./test/keys/cert.pem --key ./test/keys/key.pem -v -r ./test/resources" }, "bin": { "ldnode": "./bin/ldnode.js" From 696d6be98b1fd76e4330829183fd7f309123a758 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Wed, 5 Aug 2015 22:16:51 -0400 Subject: [PATCH 02/13] Added default error pages directory --- test/resources/errorPages/404.html | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 test/resources/errorPages/404.html diff --git a/test/resources/errorPages/404.html b/test/resources/errorPages/404.html new file mode 100644 index 000000000..9d55f67fe --- /dev/null +++ b/test/resources/errorPages/404.html @@ -0,0 +1,5 @@ + + + 404 Error Page + + From c5d45c070a3d11ec96d1a47a6b7c6001ef902191 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Wed, 5 Aug 2015 22:18:08 -0400 Subject: [PATCH 03/13] Custom error handling --- bin/ldnode.js | 8 ++++++++ index.js | 8 ++++++++ lib/error.js | 29 +++++++++++++++++++++++++++++ lib/ldp.js | 17 +++++++++++++---- 4 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 lib/error.js diff --git a/bin/ldnode.js b/bin/ldnode.js index ff50b3e9d..2aa39d3bc 100644 --- a/bin/ldnode.js +++ b/bin/ldnode.js @@ -79,6 +79,14 @@ var argv = require('nomnom') help: 'Suffix for SSE files (default: \'.events\')', abbr: 'sE' }) + .option('noErrorPages', { + full: 'no-error-pages', + help: 'Disable custom error pages (use Node.js default pages instead)' + }) + .option('errorPages', { + full: 'error-pages', + help: 'Folder from which to look for custom error pages files (files must be named .html -- eg. 500.html)' + }) .parse(); // Print version and leave diff --git a/index.js b/index.js index 79ff3b3bc..ffffdec0c 100644 --- a/index.js +++ b/index.js @@ -32,6 +32,9 @@ var putHandler = require('./lib/handlers/put.js'); var deleteHandler = require('./lib/handlers/delete.js'); var patchHandler = require('./lib/handlers/patch.js'); +// Error page handler +var errorHandler = require('./lib/error.js'); + function ldnode (argv) { var ldp = new LDP(argv); var app = express(); @@ -70,6 +73,11 @@ function createServer(argv) { var ldp = ldpApp.locals.ldp; app.use(ldp.mount, ldpApp); + //Error handling + app.use(errorHandler.handler); + + app.locals.ldp = ldp; + if (ldp && (ldp.webid || ldp.key || ldp.cert) ) { debug.settings("SSL Private Key path: " + ldp.key); debug.settings("SSL Certificate path: " + ldp.cert); diff --git a/lib/error.js b/lib/error.js new file mode 100644 index 000000000..86fe5bb11 --- /dev/null +++ b/lib/error.js @@ -0,0 +1,29 @@ +/*jslint node: true*/ +"use strict"; + +var fs = require('fs'); + +function errorPageHandler(err, req, res, next) { + var ldp = req.app.locals.ldp; + if (ldp.customErrorPages) { + var errorPage = ldp.errorPages + + err.status.toString() + '.html'; + fs.readFile(errorPage, 'utf8', function(readErr, text) { + if (readErr) { + defaultErrorHandler(err, res); + } else { + res.status(err.status); + res.send(text); + } + }); + } else { + defaultErrorHandler(err, res); + } +} + +function defaultErrorHandler(err, res) { + res.status(err.status); + res.send(err.message); +} + +exports.handler = errorPageHandler; diff --git a/lib/ldp.js b/lib/ldp.js index 8855b7ce4..9d861fc36 100644 --- a/lib/ldp.js +++ b/lib/ldp.js @@ -27,6 +27,10 @@ function LDP(argv) { ldp.cache = argv.cache; ldp.live = argv.live; ldp.root = argv.root || process.cwd(); + // Add trailing / + if (!(S(ldp.root).endsWith('/'))) { + ldp.root += '/'; + } ldp.port = argv.port; ldp.secret = argv.secret; ldp.cert = argv.cert; @@ -48,10 +52,6 @@ function LDP(argv) { ldp.suffixChanges = argv.suffixChanges || '.changes'; ldp.suffixSSE = argv.suffixSSE || '.events'; - if (!(S(ldp.root).endsWith('/'))) { - ldp.root += '/'; - } - ldp.pathFilter = regexp().start(ldp.mount).toRegExp(); ldp.xssProxy = argv.xssProxy; ldp.proxyFilter = regexp().start(ldp.xssProxy).toRegExp(); @@ -61,6 +61,15 @@ function LDP(argv) { ldp.SSEsubscriptions = {}; ldp.subscriptions = {}; + // Error pages folder + ldp.customErrorPages = !argv.noErrorPages; + if (ldp.customErrorPages) { + ldp.errorPages = argv.errorPages || ldp.root + 'errorPages/'; + if (!S(ldp.errorPages).endsWith('/')) { + ldp.errorPages += '/'; + } + } + debug.settings("mount: " + ldp.mount); debug.settings("root: " + ldp.root); debug.settings("URI path filter regexp: " + ldp.pathFilter); From c236bb1544ce9c202ba5969938e106af786317e8 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Wed, 5 Aug 2015 22:24:22 -0400 Subject: [PATCH 04/13] Modify request handlers to forward errors to custom page error handler --- lib/acl.js | 21 ++++++++++++++------- lib/handlers/delete.js | 6 ++---- lib/handlers/get.js | 41 +++++++++++++++++++++++------------------ lib/handlers/patch.js | 18 ++++++++---------- lib/handlers/post.js | 35 ++++++++++++++++++++++++----------- lib/handlers/put.js | 7 +++---- lib/header.js | 7 +++++-- lib/login.js | 5 ++++- 8 files changed, 83 insertions(+), 57 deletions(-) diff --git a/lib/acl.js b/lib/acl.js index 08797ae89..c3375a6f0 100644 --- a/lib/acl.js +++ b/lib/acl.js @@ -230,13 +230,12 @@ ACL.prototype.allowMode = function (mode, userId, aclGraph, accessType, pathUri, return callback(allowed); }); }; + /** * Callback used by allowMode. * @callback ACL~allowMode_cb * @param {Boolean} result Found valid ACL statement */ - - ACL.prototype.allowControl = function (mode, userId, aclGraph, accessType, pathUri, callback) { var acl = this; @@ -257,12 +256,12 @@ ACL.prototype.allowControl = function (mode, userId, aclGraph, accessType, pathU }, callback); }; + /** * Callback used by allowControl. * @callback ACL~allowControl_cb * @param {Boolean} result Found valid ACL statement */ - ACL.prototype.allow = function(mode, address, callback) { var ldp = this.ldp; var acl = this; @@ -384,7 +383,7 @@ ACL.prototype.fetchDocument = function(uri, callback) { // return an empty body to be parsed return cb(null, ''); } - + return ldp.readFile(documentPath, cb); }); }, @@ -510,7 +509,11 @@ function allow(mode, req, res, next) { var reqPath = res && res.locals && res.locals.path ? res.locals.path : req.path; var acl = reqToACL(req); acl.allow(mode, reqPath, function(err) { - next(err); + if (err) { + next(err); + } else { + next(); + } }); } @@ -540,7 +543,11 @@ exports.allowAppendThenWriteHandler = function(req, res, next) { } // Append failed, maybe user can write allow("Write", req, res, function(err) { - next(err); + if (err) { + return next(err); + } else { + return next(); + } }); }); @@ -549,4 +556,4 @@ exports.allowAppendThenWriteHandler = function(req, res, next) { exports.allowControlHandler = function(req, res, next) { allowIfACLEnabled("Control", req, res, next); -}; \ No newline at end of file +}; diff --git a/lib/handlers/delete.js b/lib/handlers/delete.js index 7ce64d911..555338405 100644 --- a/lib/handlers/delete.js +++ b/lib/handlers/delete.js @@ -7,7 +7,7 @@ var utils = require('../utils.js'); var metadata = require('../metadata.js'); // Delete a container or resource -function handler(req, res) { +function handler(req, res, next) { debug('DELETE -- ' + req.originalUrl); var ldp = req.app.locals.ldp; @@ -16,9 +16,7 @@ function handler(req, res) { ldp.delete(filename, function(err) { if (err) { debug("DELETE -- error: " + err); - return res - .status(err.status) - .send(err.message); + return next(err); } debug("DELETE -- Ok."); diff --git a/lib/handlers/get.js b/lib/handlers/get.js index 286f9385b..53b2d2d2e 100644 --- a/lib/handlers/get.js +++ b/lib/handlers/get.js @@ -23,7 +23,7 @@ var turtleExtension = '.ttl'; // this should be moved to options var browseSkin = 'https://linkeddata.github.io/warp/#/list/'; -function get(req, res, includeBody) { +function get(req, res, next, includeBody) { var ldp = req.app.locals.ldp; var uri = utils.uriBase(req); var filename = utils.uriToFilename(req.path, ldp.root); @@ -77,15 +77,13 @@ function get(req, res, includeBody) { // This should be implemented in LDP.prototype.get if (err && err.status === 404 && glob.hasMagic(filename)) { debug("GET/HEAD -- Glob request"); - return globHandler(req, res); + return globHandler(req, res, next); } if (err) { debug('GET/HEAD -- Read error: ' + err.status + ' ' + err.message); - return res - .status(err.status) - .send(err.message); + return next(err); } // Just return that file exists @@ -122,11 +120,11 @@ function get(req, res, includeBody) { // TODO this should be added as a middleware in the routes res.locals.turtleData = data; - return parseLinkedData(req, res); + return parseLinkedData(req, res, next); }); } -function globHandler(req, res) { +function globHandler(req, res, next) { var ldp = req.app.locals.ldp; var filename = utils.uriToFilename(req.path, ldp.root); var uri = utils.uriBase(req); @@ -139,7 +137,10 @@ function globHandler(req, res) { glob(filename, globOptions, function(err, matches) { if (err || matches.length === 0) { debug("GET/HEAD -- No files matching the pattern"); - return res.sendStatus(404); + var globErr = new Error(); + globErr.status = 404; + globErr.message = "No files matching glob pattern"; + return next(globErr); } // Matches found @@ -176,7 +177,7 @@ function globHandler(req, res) { 'text/turtle'); // TODO this should be added as a middleware in the routes res.locals.turtleData = data; - return parseLinkedData(req, res); + return parseLinkedData(req, res, next); }); }); } @@ -195,7 +196,7 @@ function aclAllow(match, req, res, callback) { }); } -function parseLinkedData(req, res) { +function parseLinkedData(req, res, next) { var ldp = req.app.locals.ldp; var filename = utils.uriToFilename(req.path, ldp.root); var uri = utils.uriBase(req); @@ -220,16 +221,20 @@ function parseLinkedData(req, res) { $rdf.parse(turtleData, resourceGraph, baseUri, 'text/turtle'); } catch (err) { debug("GET/HEAD -- Error parsing data: " + err); - return res - .status(500) - .send(err); + var parseErr = new Error(); + parseErr.status = 500; + parseErr.message = err.message; + return next(parseErr); } // Graph to `accept` type $rdf.serialize(undefined, resourceGraph, null, accept, function(err, result) { if (result === undefined || err) { debug("GET/HEAD -- Serialization error: " + err); - return res.sendStatus(500); + var serializeErr = new Error(); + serializeErr.status = 500; + serializeErr.message = err.message; + return next(serializeErr); } return res @@ -239,12 +244,12 @@ function parseLinkedData(req, res) { }); } -function getHandler(req, res) { - get(req, res, true); +function getHandler(req, res, next) { + get(req, res, next, true); } -function headHandler(req, res) { - get(req, res, false); +function headHandler(req, res, next) { + get(req, res, next, false); } exports.handler = getHandler; diff --git a/lib/handlers/patch.js b/lib/handlers/patch.js index 2af33d6d7..4a713cae7 100644 --- a/lib/handlers/patch.js +++ b/lib/handlers/patch.js @@ -9,7 +9,7 @@ var debug = require('../logging').handlers; var utils = require('../utils.js'); var subscription = require('../subscription.js'); -function handler(req, res) { +function handler(req, res, next) { var ldp = req.app.locals.ldp; debug('PATCH -- ' + req.originalUrl); debug('PATCH -- text length: ' + (req.text ? req.text.length : 'undefined2')); @@ -26,9 +26,7 @@ function handler(req, res) { if (patchContentType === 'application/sparql') { sparql(filename, targetURI, req.text, function(err, result) { if (err) { - return res - .status(err.status) - .send(err.message); + next(err); } return res.json(result); @@ -37,9 +35,7 @@ function handler(req, res) { if (patchContentType === 'application/sparql-update') { return sparqlUpdate(filename, targetURI, req.text, function (err, patchKB) { if (err) { - return res - .status(err.status) - .send(err.message); + return next(err); } if (ldp.live) { @@ -49,9 +45,11 @@ function handler(req, res) { res.send("Patch applied OK\n"); }); } else { - return res - .status(400) - .send("Sorry unknowm patch content type: " + patchContentType); + var contentErr = new Error(); + contentErr.status = 400; + contentErr.message = "Sorry unknowm patch content type: " + + patchContentType; + next(contentErr); } } // postOrPatch diff --git a/lib/handlers/post.js b/lib/handlers/post.js index 670bccac9..ad28eac53 100644 --- a/lib/handlers/post.js +++ b/lib/handlers/post.js @@ -14,7 +14,7 @@ var patch = require('./patch.js'); var ldpVocab = require('../vocab/ldp.js'); var rdfVocab = require('../vocab/rdf.js'); -function handler(req, res) { +function handler(req, res, next) { var ldp = req.app.locals.ldp; var contentType = req.get('content-type'); @@ -38,9 +38,10 @@ function handler(req, res) { contentType != 'application/nquads' && contentType != 'application/n-quads') { debug("POST -- Invalid Content Type: " + contentType); - return res - .status(415) - .send("Invalid Content Type"); + var contentErr = new Error(); + contentErr.status = 415; + contentErr.message = "Invalid Content Type"; + return next(contentErr); } @@ -50,9 +51,11 @@ function handler(req, res) { // Not a container if (containerPath[containerPath.length - 1] != '/') { debug("POST -- Requested resource is not a container"); - return res - .set('Allow', 'GET,HEAD,PUT,DELETE') - .sendStatus(405); + res.set('Allow', 'GET,HEAD,PUT,DELETE'); + var allowErr = new Error(); + allowErr.status = 405; + allowErr.message = "Requested resource is not a container"; + return next(allowErr); } debug("POST -- Content Type: " + contentType); @@ -69,7 +72,10 @@ function handler(req, res) { if (resourcePath === null) { ldp.releaseResourceUri(resourcePath); debug("POST -- URI already exists or in use"); - return res.sendStatus(400); + var resourceErr = new Error(); + resourceErr.status = 400; + resourceErr.message = "URI alredy exists or in use"; + return next(resourceErr); } // Creating a graph and add the req text @@ -90,7 +96,8 @@ function handler(req, res) { } catch (parseErr) { debug("POST -- Error parsing resource: " + parseErr); ldp.releaseResourceUri(resourcePath); - return res.sendStatus(400); + parseErr.status = 400; + return next(parseErr); } // Add header link to the resource @@ -116,7 +123,10 @@ function handler(req, res) { function containerCallback(err) { if (err) { debug("POST -- Error creating new container: " + err); - return res.sendStatus(500); + var createErr = new Error(); + createErr.status = 500; + createErr.message = "Cannot create new container"; + return next(createErr); } debug("POST -- Created new container " + resourceBaseUri); res.set('Location', resourceBaseUri); @@ -126,7 +136,10 @@ function handler(req, res) { function resourceCallback(err) { if (err) { debug("POST -- Error creating resource: " + err); - return res.sendStatus(500); + var createErr = new Error(); + createErr.status = 500; + createErr.message = "Cannot create new container"; + return next(createErr); } res.set('Location', resourceBaseUri); return res.sendStatus(201); diff --git a/lib/handlers/put.js b/lib/handlers/put.js index 08972c827..740040bb5 100644 --- a/lib/handlers/put.js +++ b/lib/handlers/put.js @@ -9,7 +9,7 @@ var debug = require('../logging').handlers; var utils = require('../utils.js'); var header = require('../header.js'); -function handler(req, res) { +function handler(req, res, next) { var ldp = req.app.locals.ldp; debug("PUT -- Request path: " + req.originalUrl); debug("PUT -- Text length: " + (req.text ? req.text.length : 'undefined')); @@ -20,9 +20,8 @@ function handler(req, res) { ldp.put(filePath, req.text, function(err) { if (err) { debug("PUT -- Write error: " + err.message); - return res - .status(err.status) - .send("Can't write file: "+ err.message); + err.message = "Can't write file: " + err.message; + return next(err); } debug("PUT -- Write Ok. Bytes written: " + req.text.length); diff --git a/lib/header.js b/lib/header.js index e6f438c0e..205aaf368 100644 --- a/lib/header.js +++ b/lib/header.js @@ -36,7 +36,10 @@ function linksHandler(req, res, next) { filename = path.join(filename, req.path); if (ldp.isMetadataFile(filename)) { debug.metadata("Trying to access metadata file as regular file."); - return res.send(404); + var metaErr = new Error(); + metaErr.status = 404; + metaErr.message = "Trying to access metadata file as regular file"; + return next(metaErr); } var fileMetadata = new metadata.Metadata(); if (S(filename).endsWith('/')) { @@ -117,4 +120,4 @@ module.exports.addLinks = addLinks; module.exports.parseMetadataFromHeader = parseMetadataFromHeader; module.exports.parseAcceptRDFHeader = parseAcceptRDFHeader; module.exports.linksHandler = linksHandler; -module.exports.negotiateContentType = negotiateContentType; \ No newline at end of file +module.exports.negotiateContentType = negotiateContentType; diff --git a/lib/login.js b/lib/login.js index c9356d9eb..b03b3e60b 100644 --- a/lib/login.js +++ b/lib/login.js @@ -40,7 +40,10 @@ function loginHandler(req, res, next) { } debug("Error processing certificate: " + message); setEmptySession(req); - return res.status(403).send(message); + var authError = new Error(); + authError.status = 403; + authError.message = message; + return next(authError); } else { req.session.userId = result; req.session.identified = true; From 9967d007e0d452be80a2392b3bf957fa164f2365 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 6 Aug 2015 12:13:11 -0400 Subject: [PATCH 05/13] Moved error.js to handlers and attached error handler to router --- index.js | 10 ++++------ lib/{ => handlers}/error.js | 0 2 files changed, 4 insertions(+), 6 deletions(-) rename lib/{ => handlers}/error.js (100%) diff --git a/index.js b/index.js index ffffdec0c..6f7fc9426 100644 --- a/index.js +++ b/index.js @@ -33,7 +33,7 @@ var deleteHandler = require('./lib/handlers/delete.js'); var patchHandler = require('./lib/handlers/patch.js'); // Error page handler -var errorHandler = require('./lib/error.js'); +var errorHandler = require('./lib/handlers/error.js'); function ldnode (argv) { var ldp = new LDP(argv); @@ -73,11 +73,6 @@ function createServer(argv) { var ldp = ldpApp.locals.ldp; app.use(ldp.mount, ldpApp); - //Error handling - app.use(errorHandler.handler); - - app.locals.ldp = ldp; - if (ldp && (ldp.webid || ldp.key || ldp.cert) ) { debug.settings("SSL Private Key path: " + ldp.key); debug.settings("SSL Certificate path: " + ldp.cert); @@ -196,6 +191,9 @@ function routes () { router.delete('/*', deleteHandler.handler); router.post('/*', postHandler.handler); router.patch('/*', patchHandler.handler); + + //Error handling + router.use(errorHandler.handler); return router; } diff --git a/lib/error.js b/lib/handlers/error.js similarity index 100% rename from lib/error.js rename to lib/handlers/error.js From c575c1a971922e69b3f6ca4a1227dd2773d9081d Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 6 Aug 2015 12:16:43 -0400 Subject: [PATCH 06/13] no-error-pages is now a flag --- bin/ldnode.js | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/ldnode.js b/bin/ldnode.js index 2aa39d3bc..f59cb240f 100644 --- a/bin/ldnode.js +++ b/bin/ldnode.js @@ -81,6 +81,7 @@ var argv = require('nomnom') }) .option('noErrorPages', { full: 'no-error-pages', + flag: true, help: 'Disable custom error pages (use Node.js default pages instead)' }) .option('errorPages', { From 672e8efab66e4f2cfedbb106cf0d00d8adc71edb Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 6 Aug 2015 12:31:33 -0400 Subject: [PATCH 07/13] Add more test error pages --- test/resources/errorPages/401.html | 5 +++++ test/resources/errorPages/403.html | 5 +++++ test/resources/errorPages/405.html | 5 +++++ test/resources/errorPages/415.html | 5 +++++ test/resources/errorPages/500.html | 5 +++++ 5 files changed, 25 insertions(+) create mode 100644 test/resources/errorPages/401.html create mode 100644 test/resources/errorPages/403.html create mode 100644 test/resources/errorPages/405.html create mode 100644 test/resources/errorPages/415.html create mode 100644 test/resources/errorPages/500.html diff --git a/test/resources/errorPages/401.html b/test/resources/errorPages/401.html new file mode 100644 index 000000000..3d05043d5 --- /dev/null +++ b/test/resources/errorPages/401.html @@ -0,0 +1,5 @@ + + + 401 Error Page + + diff --git a/test/resources/errorPages/403.html b/test/resources/errorPages/403.html new file mode 100644 index 000000000..1cef7aa16 --- /dev/null +++ b/test/resources/errorPages/403.html @@ -0,0 +1,5 @@ + + + 403 Error Page + + diff --git a/test/resources/errorPages/405.html b/test/resources/errorPages/405.html new file mode 100644 index 000000000..48d65409b --- /dev/null +++ b/test/resources/errorPages/405.html @@ -0,0 +1,5 @@ + + + 405 Error Page + + diff --git a/test/resources/errorPages/415.html b/test/resources/errorPages/415.html new file mode 100644 index 000000000..9b364da21 --- /dev/null +++ b/test/resources/errorPages/415.html @@ -0,0 +1,5 @@ + + + 415 Error Page + + diff --git a/test/resources/errorPages/500.html b/test/resources/errorPages/500.html new file mode 100644 index 000000000..64014d947 --- /dev/null +++ b/test/resources/errorPages/500.html @@ -0,0 +1,5 @@ + + + 500 Error Page + + From e7ce40ec91dde3ce2437b6ff5b9becafa984dabf Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 6 Aug 2015 19:33:41 -0400 Subject: [PATCH 08/13] Refactored code to use noErrorPage option directly instead of customPageErrors --- lib/handlers/error.js | 2 +- lib/ldp.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/handlers/error.js b/lib/handlers/error.js index 86fe5bb11..b4141261c 100644 --- a/lib/handlers/error.js +++ b/lib/handlers/error.js @@ -5,7 +5,7 @@ var fs = require('fs'); function errorPageHandler(err, req, res, next) { var ldp = req.app.locals.ldp; - if (ldp.customErrorPages) { + if (!ldp.noErrorPages) { var errorPage = ldp.errorPages + err.status.toString() + '.html'; fs.readFile(errorPage, 'utf8', function(readErr, text) { diff --git a/lib/ldp.js b/lib/ldp.js index 9d861fc36..3985c041a 100644 --- a/lib/ldp.js +++ b/lib/ldp.js @@ -62,8 +62,8 @@ function LDP(argv) { ldp.subscriptions = {}; // Error pages folder - ldp.customErrorPages = !argv.noErrorPages; - if (ldp.customErrorPages) { + ldp.noErrorPages = argv.noErrorPages; + if (!ldp.noErrorPages) { ldp.errorPages = argv.errorPages || ldp.root + 'errorPages/'; if (!S(ldp.errorPages).endsWith('/')) { ldp.errorPages += '/'; From e55202b8c09bbf46642baae6ba0213eed6f57f84 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Thu, 6 Aug 2015 19:34:35 -0400 Subject: [PATCH 09/13] Add basic page error handling tests --- package.json | 1 + test/errors.js | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 test/errors.js diff --git a/package.json b/package.json index 67a4379d1..a53290fec 100644 --- a/package.json +++ b/package.json @@ -64,6 +64,7 @@ "test-params": "./node_modules/mocha/bin/mocha ./test/params.js", "test-http": "./node_modules/mocha/bin/mocha ./test/http.js", "test-formats": "./node_modules/mocha/bin/mocha ./test/formats.js", + "test-errors": "./node_modules/mocha/bin/mocha ./test/errors.js", "ldp-webid": "node ./bin/ldnode.js --webid --cert ./test/keys/cert.pem --key ./test/keys/key.pem -v -r ./test/resources", "ldp-ssl": "node ./bin/ldnode.js --cert ./test/keys/cert.pem --key ./test/keys/key.pem -v -r ./test/resources" }, diff --git a/test/errors.js b/test/errors.js new file mode 100644 index 000000000..b9788ecb6 --- /dev/null +++ b/test/errors.js @@ -0,0 +1,64 @@ +/*jslint node: true*/ +var assert = require('chai').assert; +var fs = require('fs'); +var $rdf = require('rdflib'); +var request = require('request'); +var S = require('string'); +var supertest = require('supertest'); +var async = require('async'); + +// Helper functions for the FS +var rm = require('./test-utils').rm; +var write = require('./test-utils').write; +var cp = require('./test-utils').cp; +var read = require('./test-utils').read; + +var ldnode = require('../index'); +var ACL = require('../lib/acl').ACL; +var ns = require('../lib/vocab/ns.js').ns; + +describe('Error page tests', function() { + var errorAddress = 'http://localhost:3457/test/'; + + var errorLdp = ldnode.createServer({ + root: __dirname + '/resources', + }); + errorLdp.listen(3457); + + var errorServer = supertest(errorAddress); + + // Instance of server with error pages flag set to false + var noErrorAddress = 'http://localhost:3458/test/'; + + var noErrorLdp = ldnode.createServer({ + root: __dirname + '/resources', + noErrorPages: true + }); + noErrorLdp.listen(3458); + + var noErrorServer = supertest(errorAddress); + + + describe('Error page test', function() { + it('Should return 404 custom page if flag set to true', + function(done) { + errorServer.get('/non-existent-file.html') + .expect(/404 Error Page/) + .expect(404, done); + }); + it('Should return 404 default page if flag set to false', + function(done) { + function isDefaultErrorPage(customText) { + var handler = function (res) { + if (res.text.match(customText)){ + console.log("Not default text"); + } + }; + return handler; + } + noErrorServer.get('/non-existent-file.html') + .expect(isDefaultErrorPage('404 Error Page')) + .expect(404, done); + }); + }); +}); From 040960e05b3d8d983b72359f7b75730b65c96252 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Fri, 7 Aug 2015 13:32:36 -0400 Subject: [PATCH 10/13] Removed err check because it is not needed. --- lib/acl.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/acl.js b/lib/acl.js index c3375a6f0..980d0cd92 100644 --- a/lib/acl.js +++ b/lib/acl.js @@ -509,11 +509,7 @@ function allow(mode, req, res, next) { var reqPath = res && res.locals && res.locals.path ? res.locals.path : req.path; var acl = reqToACL(req); acl.allow(mode, reqPath, function(err) { - if (err) { - next(err); - } else { - next(); - } + return next(err); }); } @@ -543,11 +539,7 @@ exports.allowAppendThenWriteHandler = function(req, res, next) { } // Append failed, maybe user can write allow("Write", req, res, function(err) { - if (err) { - return next(err); - } else { - return next(); - } + return next(err); }); }); From 469714ce6b06125d1225715f1230608062d9bfc1 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Fri, 7 Aug 2015 13:39:02 -0400 Subject: [PATCH 11/13] Fixed misspelling --- lib/handlers/post.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/handlers/post.js b/lib/handlers/post.js index ad28eac53..9674b5e61 100644 --- a/lib/handlers/post.js +++ b/lib/handlers/post.js @@ -74,7 +74,7 @@ function handler(req, res, next) { debug("POST -- URI already exists or in use"); var resourceErr = new Error(); resourceErr.status = 400; - resourceErr.message = "URI alredy exists or in use"; + resourceErr.message = "URI already exists or in use"; return next(resourceErr); } From 3b5d10384f5b909b2a68d50d784c5762150b4cb3 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Fri, 7 Aug 2015 14:01:36 -0400 Subject: [PATCH 12/13] Tests now read the file instead of comparing againts fixed string --- test/errors.js | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/test/errors.js b/test/errors.js index b9788ecb6..18141119b 100644 --- a/test/errors.js +++ b/test/errors.js @@ -40,24 +40,26 @@ describe('Error page tests', function() { describe('Error page test', function() { + var file404 = 'errorPages/404.html'; + function defaultErrorPage(filepath, expected) { + var handler = function (res) { + var errorFile = read(filepath); + if (res.text === errorFile && !expected){ + console.log("Not default text"); + } + }; + return handler; + } it('Should return 404 custom page if flag set to true', function(done) { - errorServer.get('/non-existent-file.html') - .expect(/404 Error Page/) + errorServer.get('non-existent-file.html') + .expect(defaultErrorPage(file404, true)) .expect(404, done); }); it('Should return 404 default page if flag set to false', function(done) { - function isDefaultErrorPage(customText) { - var handler = function (res) { - if (res.text.match(customText)){ - console.log("Not default text"); - } - }; - return handler; - } - noErrorServer.get('/non-existent-file.html') - .expect(isDefaultErrorPage('404 Error Page')) + noErrorServer.get('non-existent-file.html') + .expect(defaultErrorPage(file404, false)) .expect(404, done); }); }); From 9349c1b30cd7ce9be1ffc8fcf03ed6a645d382d9 Mon Sep 17 00:00:00 2001 From: Martin Martinez Rivera Date: Fri, 7 Aug 2015 19:04:47 -0400 Subject: [PATCH 13/13] No error folder default. Folder must be explicitly stated. --- lib/ldp.js | 7 +++++-- test/errors.js | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/ldp.js b/lib/ldp.js index 3985c041a..054985101 100644 --- a/lib/ldp.js +++ b/lib/ldp.js @@ -64,8 +64,11 @@ function LDP(argv) { // Error pages folder ldp.noErrorPages = argv.noErrorPages; if (!ldp.noErrorPages) { - ldp.errorPages = argv.errorPages || ldp.root + 'errorPages/'; - if (!S(ldp.errorPages).endsWith('/')) { + ldp.errorPages = argv.errorPages; + if (!ldp.errorPages) { + // For now disable error pages if errorPages parameter is not explicitly passed + ldp.noErrorPages = true; + } else if (!S(ldp.errorPages).endsWith('/')) { ldp.errorPages += '/'; } } diff --git a/test/errors.js b/test/errors.js index 18141119b..69f0e230c 100644 --- a/test/errors.js +++ b/test/errors.js @@ -22,6 +22,7 @@ describe('Error page tests', function() { var errorLdp = ldnode.createServer({ root: __dirname + '/resources', + errorPages: __dirname + '/resources/errorPages' }); errorLdp.listen(3457);