From 5646680e5efe1f2f220193db06dccffc210ed621 Mon Sep 17 00:00:00 2001 From: David Cheung Date: Fri, 27 May 2016 11:04:20 -0400 Subject: [PATCH 1/2] HTML response for accepted headers --- README.md | 13 ++- lib/content-negotiation.js | 85 ++++++++++++++++++++ lib/handler.js | 10 +-- lib/send-html.js | 46 +++++++++++ package.json | 7 ++ test/handler.test.js | 161 ++++++++++++++++++++++++++++++++++++- views/default-error.ejs | 25 ++++++ views/style.css | 31 +++++++ 8 files changed, 365 insertions(+), 13 deletions(-) create mode 100644 lib/content-negotiation.js create mode 100644 lib/send-html.js create mode 100644 views/default-error.ejs create mode 100644 views/style.css diff --git a/README.md b/README.md index 2658fd6..710b7bf 100644 --- a/README.md +++ b/README.md @@ -8,10 +8,6 @@ Error handler for use in development (debug) and production environments. - When in debug mode, detailed information such as stack traces are returned in the HTTP responses. -JSON is the only supported response format at this time. - -*There are plans to support other formats such as Text, HTML, and XML.* - ## Install ```bash @@ -47,6 +43,15 @@ In LoopBack applications, add the following entry to your } ``` +## Content Type + +Depending on the request header's `Accepts`, response will be returned in + the corresponding content-type, current supported types include: +- JSON (`json`/`application/json`) +- HTML (`html`/`text/html`) + +*There are plans to support other formats such as Text and XML.* + ## Options #### debug diff --git a/lib/content-negotiation.js b/lib/content-negotiation.js new file mode 100644 index 0000000..6222025 --- /dev/null +++ b/lib/content-negotiation.js @@ -0,0 +1,85 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-error-handler +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +var accepts = require('accepts'); +var debug = require('debug')('strong-error-handler:http-response'); +var sendJson = require('./send-json'); +var sendHtml = require('./send-html'); +var util = require('util'); + +module.exports = negotiateContentProducer; + +/** + * Handles req.accepts and req.query._format and options.defaultType + * to resolve the correct operation + * + * @param req request object + * @param {Object} options options of strong-error-handler + * @returns {Function} Opeartion function with signature `fn(res, data)` + */ +function negotiateContentProducer(req, options) { + var SUPPORTED_TYPES = [ + 'application/json', 'json', + 'text/html', 'html', + ]; + + options = options || {}; + var defaultType = 'json'; + + // checking if user provided defaultType is supported + if (options.defaultType) { + if (SUPPORTED_TYPES.indexOf(options.defaultType) > -1) { + debug('Accepting options.defaultType `%s`', options.defaultType); + defaultType = options.defaultType; + } else { + debug('defaultType: `%s` is not supported, ' + + 'falling back to defaultType: `%s`', options.defaultType, defaultType); + } + } + + // decide to use resolvedType or defaultType + // Please note that accepts assumes the order of content-type is provided + // in the priority returned + // example + // Accepts: text/html, */*, application/json ---> will resolve as text/html + // Accepts: application/json, */*, text/html ---> will resolve as application/json + // Accepts: */*, application/json, text/html ---> will resolve as application/json + // eg. Chrome accepts defaults to `text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*` + // In this case `resolvedContentType` will result as: `text/html` due to the order given + var resolvedContentType = accepts(req).types(SUPPORTED_TYPES); + debug('Resolved content-type', resolvedContentType); + var contentType = resolvedContentType || defaultType; + + // to receive _format from user's url param to overide the content type + // req.query (eg /api/Users/1?_format=json will overide content negotiation + // https://github.com/strongloop/strong-remoting/blob/ac3093dcfbb787977ca0229b0f672703859e52e1/lib/http-context.js#L643-L645 + var query = req.query || {}; + if (query._format) { + if (SUPPORTED_TYPES.indexOf(query._format) > -1) { + contentType = query._format; + } else { + // format passed through query but not supported + var msg = util.format('Response _format "%s" is not supported' + + 'used "%s" instead"', query._format, defaultType); + res.header('X-Warning', msg); + debug(msg); + } + } + + debug('Content-negotiation: req.headers.accept: `%s` Resolved as: `%s`', + req.headers.accept, contentType); + return resolveOperation(contentType); +} + +function resolveOperation(contentType) { + switch (contentType) { + case 'application/json': + case 'json': + return sendJson; + case 'text/html': + case 'html': + return sendHtml; + } +} diff --git a/lib/handler.js b/lib/handler.js index bba6ef7..fb6196d 100644 --- a/lib/handler.js +++ b/lib/handler.js @@ -9,7 +9,7 @@ var buildResponseData = require('./data-builder'); var debug = require('debug')('strong-error-handler'); var format = require('util').format; var logToConsole = require('./logger'); -var sendJson = require('./send-json'); +var negotiateContentProducer = require('./content-negotiation'); function noop() { } @@ -52,11 +52,7 @@ exports = module.exports = function createStrongErrorHandler(options) { res.setHeader('X-Content-Type-Options', 'nosniff'); res.statusCode = data.statusCode; - // TODO: negotiate the content-type, take into account options.defaultType - // For now, we always return JSON. See - // - https://github.com/strongloop/strong-error-handler/issues/4 - // - https://github.com/strongloop/strong-error-handler/issues/5 - // - https://github.com/strongloop/strong-error-handler/issues/6 - sendJson(res, data); + var sendResponse = negotiateContentProducer(req, options); + sendResponse(res, data); }; }; diff --git a/lib/send-html.js b/lib/send-html.js new file mode 100644 index 0000000..4b00ca0 --- /dev/null +++ b/lib/send-html.js @@ -0,0 +1,46 @@ +// Copyright IBM Corp. 2016. All Rights Reserved. +// Node module: strong-error-handler +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +var ejs = require('ejs'); +var fs = require('fs'); +var path = require('path'); + +var assetDir = path.resolve(__dirname, '../views'); +var compiledTemplates = { + // loading default template and stylesheet + default: loadDefaultTemplates(), +}; + +module.exports = sendHtml; + +function sendHtml(res, data, options) { + var toRender = {options: {}, data: data}; + // TODO: ability to call non-default template functions from options + var body = compiledTemplates.default(toRender); + sendReponse(res, body); +} + +/** + * Compile and cache the file with the `filename` key in options + * + * @param filepath (description) + * @returns {Function} render function with signature fn(data); + */ +function compileTemplate(filepath) { + var options = {cache: true, filename: filepath}; + fileContent = fs.readFileSync(filepath, 'utf8'); + return ejs.compile(fileContent, options); +} + +// loads and cache default error templates +function loadDefaultTemplates() { + var defaultTemplate = path.resolve(assetDir, 'default-error.ejs'); + return compileTemplate(defaultTemplate); +} + +function sendReponse(res, body) { + res.setHeader('Content-Type', 'text/html; charset=utf-8'); + res.end(body); +} diff --git a/package.json b/package.json index 71cab6c..46fd561 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,9 @@ "posttest": "npm run lint" }, "dependencies": { + "accepts": "^1.3.3", "debug": "^2.2.0", + "ejs": "^2.4.2", "http-status": "^0.2.2" }, "devDependencies": { @@ -22,6 +24,11 @@ "eslint": "^2.5.3", "eslint-config-loopback": "^3.0.0", "mocha": "^2.1.0", + "parseurl": "^1.3.1", + "qs": "^6.2.0", "supertest": "^1.1.0" + }, + "browser": { + "strong-error-handler": false } } diff --git a/test/handler.test.js b/test/handler.test.js index 4571445..7e69fbb 100644 --- a/test/handler.test.js +++ b/test/handler.test.js @@ -9,6 +9,8 @@ var cloneAllProperties = require('../lib/clone.js'); var debug = require('debug')('test'); var expect = require('chai').expect; var http = require('http'); +var parseParam = require('qs').parse; +var parseUrl = require('parseurl'); var strongErrorHandler = require('..'); var supertest = require('supertest'); var util = require('util'); @@ -361,6 +363,153 @@ describe('strong-error-handler', function() { .expect('Content-Type', /^application\/json/); } }); + + context('HTML response', function() { + it('contains all error properties when debug=true', function(done) { + var error = new ErrorWithProps({ + message: 'a test error message', + details: 'some details', + extra: 'sensitive data', + }); + error.statusCode = 500; + givenErrorHandlerForError(error, {debug: true}); + requestHTML() + .expect(500) + .expect(/ErrorWithProps<\/title>/) + .expect(/500(.*?)a test error message/) + .expect(/extra(.*?)sensitive data/) + .expect(/details(.*?)some details/) + .expect(/id="stacktrace"(.*?)ErrorWithProps: a test error message/, + done); + }); + + it('contains subset of properties when status=4xx', function(done) { + var error = new ErrorWithProps({ + name: 'ValidationError', + message: 'The model instance is not valid.', + statusCode: 422, + details: 'some details', + extra: 'sensitive data', + }); + givenErrorHandlerForError(error, {debug: false}); + requestHTML() + .end(function(err, res) { + expect(res.statusCode).to.eql(422); + var body = res.error.text; + expect(body).to.match(/some details/); + expect(body).to.not.match(/sensitive data/); + expect(body).to.match(/<title>ValidationError<\/title>/); + expect(body).to.match(/422(.*?)The model instance is not valid./); + done(); + }); + }); + + it('contains only safe info when status=5xx', function(done) { + // Mock an error reported by fs.readFile + var error = new ErrorWithProps({ + name: 'Error', + message: 'ENOENT: no such file or directory, open "/etc/passwd"', + errno: -2, + code: 'ENOENT', + syscall: 'open', + path: '/etc/password', + }); + givenErrorHandlerForError(error); + + requestHTML() + .end(function(err, res) { + expect(res.statusCode).to.eql(500); + var body = res.error.text; + expect(body).to.not.match(/\/etc\/password/); + expect(body).to.not.match(/-2/); + expect(body).to.not.match(/ENOENT/); + // only have the following + expect(body).to.match(/<title>Internal Server Error<\/title>/); + expect(body).to.match(/500(.*?)Internal Server Error/); + done(); + }); + }); + + function requestHTML(url) { + return request.get(url || '/') + .set('Accept', 'text/html') + .expect('Content-Type', /^text\/html/); + } + }); + + context('Content Negotiation', function() { + it('defaults to json without options', function(done) { + givenErrorHandlerForError(new Error('Some error'), {}); + request.get('/') + .set('Accept', '*/*') + .expect('Content-Type', /^application\/json/, done); + }); + + it('honors accepted content-type', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'application/json', + }); + request.get('/') + .set('Accept', 'text/html') + .expect('Content-Type', /^text\/html/, done); + }); + + it('honors order of accepted content-type', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'text/html', + }); + request.get('/') + // `application/json` will be used because its provided first + .set('Accept', 'application/json, text/html') + .expect('Content-Type', /^application\/json/, done); + }); + + it('honors order of accepted content-types of text/html', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'application/json', + }); + request.get('/') + // text/html will be used because its provided first + .set('Accept', 'text/html, application/json') + .expect('Content-Type', /^text\/html/, done); + }); + + it('picks first supported type upon multiple accepted', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'application/json', + }); + request.get('/') + .set('Accept', '*/*, not-supported, text/html, application/json') + .expect('Content-Type', /^text\/html/, done); + }); + + it('falls back for unsupported option.defaultType', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'unsupported', + }); + request.get('/') + .set('Accept', '*/*') + .expect('Content-Type', /^application\/json/, done); + }); + + it('returns defaultType for unsupported type', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'text/html', + }); + request.get('/') + .set('Accept', 'unsupported/type') + .expect('Content-Type', /^text\/html/, done); + }); + + it('supports query _format', function(done) { + givenErrorHandlerForError(new Error('Some error'), { + defaultType: 'text/html', + }); + request.get('/?_format=html') + .set('Accept', 'application/json') + .expect('Content-Type', /^text\/html/, done); + }); + }); }); var _httpServer, _requestHandler, request; @@ -381,11 +530,19 @@ function givenErrorHandlerForError(error, options) { var handler = strongErrorHandler(options); _requestHandler = function(req, res, next) { - debug('Invoking strong-error-handler'); - handler(error, req, res, next); + queryMiddleware(req, res, function() { + debug('Invoking strong-error-handler'); + handler(error, req, res, next); + }); }; } +function queryMiddleware(req, res, next) { + var queryString = parseUrl(req).query; + req.query = parseParam(queryString); + next(); +} + function setupHttpServerAndClient(done) { _httpServer = http.createServer(function(req, res) { if (!_requestHandler) { diff --git a/views/default-error.ejs b/views/default-error.ejs new file mode 100644 index 0000000..26f2d9b --- /dev/null +++ b/views/default-error.ejs @@ -0,0 +1,25 @@ +<html> + <head> + <meta charset='utf-8'> + <title><%- data.name || data.message %> + + + +
+

<%- data.name %>

+

<%- data.statusCode %> <%- data.message %>

+ <% + // display all the non-standard properties + var standardProps = ['name', 'statusCode', 'message', 'stack']; + for (var prop in data) { + if (standardProps.indexOf(prop) == -1 && data[prop]) { %> +
<%- prop %>: <%- data[prop] %>
+ <% } + } + if (data.stack) { %> + 
<%- data.stack %>
+ <% } + %> +
+ + diff --git a/views/style.css b/views/style.css new file mode 100644 index 0000000..e4c170f --- /dev/null +++ b/views/style.css @@ -0,0 +1,31 @@ +* { + margin: 0; + padding: 0; + outline: 0; +} +body { + padding: 80px 100px; + font: 13px "Helvetica Neue", "Lucida Grande", "Arial"; + background: #ECE9E9 -webkit-gradient(linear, 0% 0%, 0% 100%, from(#fff), to(#ECE9E9)); + background: #ECE9E9 -moz-linear-gradient(top, #fff, #ECE9E9); + background-repeat: no-repeat; + color: #555; + -webkit-font-smoothing: antialiased; +} +h1, h2 { + font-size: 22px; + color: #343434; +} +h1 em, h2 em { + padding: 0 5px; + font-weight: normal; +} +h1 { + font-size: 60px; +} +h2 { + margin: 10px 0; +} +ul li { + list-style: none; +} From 4b29acdcb0f34b46bf11a6450c2d0d43d69c3b8f Mon Sep 17 00:00:00 2001 From: David Cheung Date: Fri, 3 Jun 2016 12:21:21 -0400 Subject: [PATCH 2/2] Test with express instead of http server --- package.json | 3 +-- test/handler.test.js | 57 ++++++++++++++++++-------------------------- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/package.json b/package.json index 46fd561..28921ca 100644 --- a/package.json +++ b/package.json @@ -23,9 +23,8 @@ "chai": "^2.1.1", "eslint": "^2.5.3", "eslint-config-loopback": "^3.0.0", + "express": "^4.13.4", "mocha": "^2.1.0", - "parseurl": "^1.3.1", - "qs": "^6.2.0", "supertest": "^1.1.0" }, "browser": { diff --git a/test/handler.test.js b/test/handler.test.js index 7e69fbb..0dad7c9 100644 --- a/test/handler.test.js +++ b/test/handler.test.js @@ -8,9 +8,7 @@ var cloneAllProperties = require('../lib/clone.js'); var debug = require('debug')('test'); var expect = require('chai').expect; -var http = require('http'); -var parseParam = require('qs').parse; -var parseUrl = require('parseurl'); +var express = require('express'); var strongErrorHandler = require('..'); var supertest = require('supertest'); var util = require('util'); @@ -512,7 +510,7 @@ describe('strong-error-handler', function() { }); }); -var _httpServer, _requestHandler, request; +var app, _requestHandler, request; function resetRequestHandler() { _requestHandler = null; } @@ -530,21 +528,14 @@ function givenErrorHandlerForError(error, options) { var handler = strongErrorHandler(options); _requestHandler = function(req, res, next) { - queryMiddleware(req, res, function() { - debug('Invoking strong-error-handler'); - handler(error, req, res, next); - }); + debug('Invoking strong-error-handler'); + handler(error, req, res, next); }; } -function queryMiddleware(req, res, next) { - var queryString = parseUrl(req).query; - req.query = parseParam(queryString); - next(); -} - function setupHttpServerAndClient(done) { - _httpServer = http.createServer(function(req, res) { + app = express(); + app.use(function(req, res, next) { if (!_requestHandler) { var msg = 'Error handler middleware was not setup in this test'; console.error(msg); @@ -553,31 +544,29 @@ function setupHttpServerAndClient(done) { res.end(msg); return; } - - _requestHandler(req, res, function(err) { - console.log('unexpected: strong-error-handler called next with ' - (err && (err.stack || err)) || 'no error'); - res.statusCode = 500; - res.setHeader('Content-Type', 'text/plain; charset=utf-8'); - res.end(err ? - 'Unhandled strong-error-handler error:\n' + (err.stack || err) : - 'The error was silently discared by strong-error-handler'); - }); + _requestHandler(req, res, warnUnhandledError); }); - _httpServer.once('error', function(err) { + app.listen(0, function() { + var url = 'http://127.0.0.1:' + this.address().port; + debug('Test server listening on %s', url); + request = supertest(app); + done(); + }) + .once('error', function(err) { debug('Cannot setup HTTP server: %s', err.stack); done(err); }); +} - _httpServer.once('listening', function() { - var url = 'http://127.0.0.1:' + this.address().port; - debug('Test server listening on %s', url); - request = supertest(url); - done(); - }); - - _httpServer.listen(0, '127.0.0.1'); +function warnUnhandledError(err) { + console.log('unexpected: strong-error-handler called next with ' + (err && (err.stack || err)) || 'no error'); + res.statusCode = 500; + res.setHeader('Content-Type', 'text/plain; charset=utf-8'); + res.end(err ? + 'Unhandled strong-error-handler error:\n' + (err.stack || err) : + 'The error was silently discared by strong-error-handler'); } function ErrorWithProps(props) {