Fixed utf8 handling. Fixed long payloads issue. Updated docsand tests

.travis.yml

@@ -20,7 +20,7 @@ script:
   - node --version
   - npm --version
   # linting
-  - npm run lint
+  - npm run ci-lint
   # compatibility testing
   - npm run compat
 

README.md

@@ -15,7 +15,7 @@ A library that allows other projects to be agnostic of particular http server im
 *Notice of change of ownership: Starting version 1.0.0 this package has changed it's owner and goals. Old version (0.0.0) is still available on npm via `npm install [email protected]` or on [github](https://github.com/dtudury/agnostic). Thank you.*
 
 
-| node / libs |  express |        restify |                                         hapi | http |
+| node / libs |  express | restify        | hapi                                         | http |
 | :--         | :--      | :--            | :--                                          | :--  |
 | v0.12       | 3.x, 4.x |  2.x, 3.x, 4.x | 8.x, 9.x, 10.x                               |  ✓   |
 | io.js       | 3.x, 4.x |  2.x, 3.x, 4.x | 8.x, 9.x, 10.x                               |  ✓   |
@@ -39,11 +39,20 @@ var agnostic = require('agnostic');
 
 module.exports = agnostic(myRequestHandler);
 
+/**
+ * Does cool things
+ *
+ * @param {EventEmitter} request - request object, mimicking IncomingMessage
+ * @param {Function} respond - callback to respond to the request
+ */
 function myRequestHandler(request, respond)
 {
   // do cool things
+  // `request.body` - parsed request body
+  // `request.query` - parsed query string
   // `respond` is a function with the following signature:
   // `respond([code], [content[, options]]);`
+  respond(200, 'Received hit to ' + request.url, {headers: {'X-Powered-By': 'AllCoolThings'}});
 }
 ```
 

appveyor.yml

@@ -5,16 +5,14 @@ environment:
     - nodejs_version: '4'
     - nodejs_version: '5'
     - nodejs_version: '6'
-platform:
-  - x86
-  - x64
 install:
-  - ps: Install-Product node $env:nodejs_version $env:platform
+  - ps: Install-Product node $env:nodejs_version
   - npm install
 test_script:
   - node --version
   - npm --version
   - npm run compat
 build: off
+version: "{build}"
 matrix:
   fast_finish: true

index.js

@@ -83,14 +83,19 @@ function requestHandlerAdapter(requestHandler, rawRequest, rawResponse)
   // invoke chosen adapter with normalized body
   normalize.request.call(this, rawRequest, function(error, request)
   {
+    var adaptedRequest, normalizedResponse;
+
     if (error)
     {
       // terminate earlier – as input error
-      normalize.response.call(this, adapter.response, rawResponse, request)(400);
+      normalize.response.call(this, adapter.response, rawResponse, request)(error.statusCode || error.status || 400, error.message);
       return;
     }
 
-    requestHandler(adapter.request.call(this, request, rawRequest), normalize.response.call(this, adapter.response, rawResponse, request));
+    adaptedRequest     = adapter.request.call(this, request, rawRequest);
+    normalizedResponse = normalize.response.call(this, adapter.response, rawResponse, request);
+
+    requestHandler(adaptedRequest, normalizedResponse);
 
   }.bind(this));
 }

lib/normalize.js

@@ -65,7 +65,7 @@ function normalizeRequest(rawRequest, callback)
 
   rawBody(rawRequest, options, function(error, body)
   {
-    if (error) return callback(error);
+    if (error) return callback(error, request);
 
     parseBody.call(this, body, rawRequest.headers['content-type'] || this.request.contentType, function(err, parsed)
     {
@@ -207,7 +207,7 @@ function wrapResponse(adapter, response, request, code, content, options)
       headers: {
         'content-type': contentMimeType,
         // TODO: multibyte and Buffers
-        'content-length': content.length
+        'content-length': Buffer.byteLength(content, 'utf8')
       }}, options);
   }
 

package.json

@@ -6,7 +6,11 @@
   "scripts": {
     "lint": "eslint *.js adapters/*.js lib/*.js test/**/*.js",
     "test": "tape test/test-*.js | tap-spec",
+    "precover": "rimraf coverage",
     "cover": "istanbul cover tape -- test/test-*.js | tap-spec",
+    "postcover": "istanbul report --root compat-coverage --dir coverage --verbose json lcov text",
+    "ci-test": "npm run cover",
+    "ci-lint": "is-node-modern && npm run lint || is-node-not-modern",
     "precompat": "rimraf coverage compat-coverage",
     "compat": "node compatibility.js",
     "postcompat": "istanbul report --root compat-coverage --dir coverage --verbose json lcov text",
@@ -50,13 +54,14 @@
     "asynckit": "^0.4.0",
     "batcher": "^2.0.0",
     "body-parser": "^1.15.2",
-    "coveralls": "^2.11.12",
-    "eslint": "^2.13.1",
+    "coveralls": "^2.11.14",
+    "eslint": "^3.6.1",
     "express": "^4.14.0",
-    "glob": "^7.0.6",
-    "hapi": "^15.0.1",
-    "hyperquest": "^2.0.0",
+    "glob": "^7.1.0",
+    "hapi": "^15.1.1",
+    "hyperquest": "^2.1.0",
     "in-publish": "^2.0.0",
+    "is-node-modern": "^1.0.0",
     "istanbul": "^0.4.5",
     "pre-commit": "^1.1.3",
     "restify": "^4.1.1",

test/common.js

@@ -1,10 +1,15 @@
 var path       = require('path')
   , glob       = require('glob')
+  , merge      = require('deeply')
   , rawBody    = require('raw-body')
   , asynckit   = require('asynckit')
   , hyperquest = require('hyperquest')
+  , agnostic   = require('../')
   ;
 
+// augment agnostic defaults for tests
+agnostic.defaults = merge(agnostic.defaults, {request: {bodyMaxLength: 100}});
+
 // expose suite methods
 var common = module.exports =
 {
@@ -46,11 +51,12 @@ function requestHandler(request, respond)
  * Runs all the requests through
  *
  * @this  test
+ * @param {string} server - name of the server it's being run with
  * @param {function} callback - invoked after all requests finished
  */
-function sendAllRequests(callback)
+function sendAllRequests(server, callback)
 {
-  asynckit.parallel(common.requests, sendRequest.bind(this), callback);
+  asynckit.parallel(common.requests, sendRequest.bind(this, server), callback);
 }
 
 /**
@@ -73,14 +79,17 @@ function sendRequestByName(name, callback)
  * to the simulated backend
  *
  * @this  test
+ * @param {string} server - name of the server it's being run with
  * @param {object} config - config for the request
  * @param {string} id - name of the config
  * @param {function} callback - invoke on response from the simulated server
  */
-function sendRequest(config, id, callback)
+function sendRequest(server, config, id, callback)
 {
   var body
     , request
+      // check for server specific config or use common one
+    , expected = config['expected.' + server] || config.expected
     , url     = path.join(common.server.endpoint, id) + (config.query || '')
     , options = {
       method  : config.method,
@@ -91,24 +100,24 @@ function sendRequest(config, id, callback)
   if (['HEAD', 'GET'].indexOf(config.method) == -1)
   {
     body = typeof config.body == 'string' ? config.body : JSON.stringify(config.body);
-    options.headers['content-length'] = body.length;
+    options.headers['content-length'] = Buffer.byteLength(body, 'utf8');
   }
 
   request = hyperquest('http://localhost:' + common.server.port + url, options, function(error, response)
   {
     this.error(error);
 
-    this.equal(response.statusCode, config.expected.status, 'expected response with the ' + config.expected.status + ' status code');
+    this.equal(response.statusCode, expected.status, 'expected response with the ' + expected.status + ' status code');
 
-    if (config.expected.headers)
+    if (expected.headers)
     {
-      Object.keys(config.expected.headers).forEach(function(name)
+      Object.keys(expected.headers).forEach(function(name)
       {
-        this.ok(response.headers[name].indexOf(config.expected.headers[name]) != -1, 'expect `' + name + '` header to contain `' + config.expected.headers[name] + '`');
+        this.ok(response.headers[name].indexOf(expected.headers[name]) != -1, 'expect `' + name + '` header to contain `' + expected.headers[name] + '`');
       }.bind(this));
     }
 
-    if (!('body' in config.expected))
+    if (!('body' in expected))
     {
       // shortcut here
       callback(null, response);
@@ -122,7 +131,7 @@ function sendRequest(config, id, callback)
     {
       this.error(err);
 
-      this.equal(content, config.expected.body, 'expect content to be the same with body');
+      this.equal(content, expected.body, 'expect content to be the same with body');
 
       // everything is clear
       callback(null, response);

test/fixtures/post_200_utf8.js

@@ -0,0 +1,27 @@
+module.exports = {
+  'method': 'POST',
+  'headers': {
+    'host': 'localhost',
+    'accept': '*/*',
+    'content-type': 'application/json'
+  },
+  'body': {
+    'message': {
+      'text': 'the naïve assumption'
+    }
+  },
+
+  requestHandler: function(req, res)
+  {
+    res(200, {object: req.body.message.text});
+  },
+
+  'expected':
+  {
+    'status': 200,
+    'body'  : '{"object":"the naïve assumption"}',
+    'headers': {
+      'content-type': 'application/json'
+    }
+  }
+};

test/fixtures/post_400_too_long.js

@@ -0,0 +1,24 @@
+module.exports = {
+  'method': 'POST',
+  'headers': {
+    'host': 'localhost'
+  },
+  'body': '{ body payload is too long, but still handled properly and with a meaningful response code and human readable error message }',
+
+  requestHandler: function(req, res)
+  {
+    res(200, {should_not: 'been here'});
+  },
+
+  'expected':
+  {
+    'status': 413,
+    'body': 'request entity too large'
+  },
+
+  'expected.hapi':
+  {
+    'status': 400,
+    'body': '{"statusCode":400,"error":"Bad Request","message":"Payload content length greater than maximum allowed: 100"}'
+  }
+};

test/test-express-middleware.js

@@ -4,7 +4,7 @@ var express    = require('express')
   , common     = require('./common.js')
   , agnostic   = require('../')
 
-  , bodyParserOptions = { limit: 1024 * 1000 }
+  , bodyParserOptions = { limit: 100 }
   ;
 
 tape.test('express as middleware, no parser', function(t)
@@ -22,7 +22,7 @@ tape.test('express as middleware, no parser', function(t)
   // start the server
   server = app.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'express', function(error, responded)
     {
       t.error(error);
 
@@ -55,7 +55,7 @@ tape.test('express as middleware, with bodyParser middleware', function(t)
   // start the server
   server = app.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'express', function(error, responded)
     {
       t.error(error);
 

test/test-express.js

@@ -4,7 +4,7 @@ var express    = require('express')
   , common     = require('./common.js')
   , agnostic   = require('../')
 
-  , bodyParserOptions = { limit: 1024 * 1000 }
+  , bodyParserOptions = { limit: 100 }
   ;
 
 tape.test('express as route, no parser', function(t)
@@ -22,7 +22,7 @@ tape.test('express as route, no parser', function(t)
   // start the server
   server = app.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'express', function(error, responded)
     {
       t.error(error);
 
@@ -55,7 +55,7 @@ tape.test('express as route, with bodyParser middleware', function(t)
   // start the server
   server = app.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'express', function(error, responded)
     {
       t.error(error);
 

test/test-hapi.js

@@ -16,18 +16,24 @@ tape.test('hapi as route, with default body parser', function(t)
   {
     // plug-in fbbot
     server.route({
-      method : ['GET', 'POST'], // Hapi uses GET handler for HEAD requests
+      method : 'GET', // Hapi uses GET handler for HEAD requests
       path   : [common.server.endpoint, id].join('/'),
       handler: agnostic(common.requests[id].requestHandler)
     });
+    server.route({
+      method : 'POST',
+      path   : [common.server.endpoint, id].join('/'),
+      handler: agnostic(common.requests[id].requestHandler),
+      config : {payload: {maxBytes: 100}}
+    });
   });
 
   // start the server
   server.start(function(err)
   {
     t.error(err);
 
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'hapi', function(error, responded)
     {
       t.error(error);
 

test/test-http.js

@@ -30,7 +30,7 @@ tape.test('http.createServer, no parser', function(t)
   // start the server
   server.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'http', function(error, responded)
     {
       t.error(error);
 

test/test-restify.js

@@ -3,7 +3,7 @@ var restify  = require('restify')
   , common   = require('./common.js')
   , agnostic = require('../')
 
-  , bodyParserOptions = { maxBodySize: 1024 * 1000 }
+  , bodyParserOptions = { maxBodySize: 100 }
   ;
 
 // Restify options
@@ -27,7 +27,7 @@ tape.test('restify as route, no parser', function(t)
   // start the server
   server.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'restify', function(error, responded)
     {
       t.error(error);
 
@@ -62,7 +62,7 @@ tape.test('restify as route, with bodyParser middleware', function(t)
   // start the server
   server.listen(common.server.port, function()
   {
-    common.sendAllRequests.call(t, function(error, responded)
+    common.sendAllRequests.call(t, 'restify', function(error, responded)
     {
       t.error(error);