Remove destroy, encoder, error-inject, escape-html, koa-is-json, on-finished, type-is and vary

lib/application.js

@@ -6,10 +6,10 @@
  */
 
 const debug = require('debug-ms')('koa:application');
-const onFinished = require('on-finished');
+const onFinished = require('./onfinish');
 const response = require('./response');
 const compose = require('koa-compose');
-const isJSON = require('koa-is-json');
+const isJSON = require('./isjson');
 const context = require('./context');
 const request = require('./request');
 const statuses = require('./statuses');

lib/isjson.js

@@ -0,0 +1,13 @@
+/**
+ * Check if `body` should be interpreted as json.
+ */
+
+function isJSON(body) {
+  if (!body) return false;
+  if ('string' == typeof body) return false;
+  if ('function' == typeof body.pipe) return false;
+  if (Buffer.isBuffer(body)) return false;
+  return true;
+}
+
+module.exports = isJSON;

lib/onfinish.js

@@ -0,0 +1,74 @@
+/**
+ * Call callback when request finished. Lifted off of
+ * npm on-finished with slight optimizations.
+ */
+
+module.exports = function onFinished(msg, callback) {
+  let alreadyFinished = false;
+
+  // Make sure it hasn't finished already.
+  // Although I highly doubt this code is necessary.
+  if (typeof msg.finished === 'boolean') {
+    alreadyFinished = msg.finished || (msg.socket && !msg.socket.writable);
+  } else if (typeof msg.complete === 'boolean') {
+    alreadyFinished = msg.upgrade || !msg.socket || !msg.socket.readable || (msg.complete && !msg.readable);
+  } else {
+    // We don't support this object so end immediately
+    alreadyFinished = true;
+  }
+
+  if (alreadyFinished) {
+    return setImmediate(callback, null, msg);
+  }
+
+  if (msg.__onFinished) {
+    return msg.__onFinished.push(callback);
+  }
+  msg.__onFinished = [callback];
+
+  let socket = null;
+  let finished = false;
+
+  function onFinish(error) {
+    if (finished) return;
+
+    msg.removeListener('end', onFinish);
+    msg.removeListener('finish', onFinish);
+
+    if (socket) {
+      socket.removeListener('error', onFinish);
+      socket.removeListener('close', onFinish);
+    }
+
+    socket = null;
+    finished = true;
+
+    msg.__onFinished.forEach(cb => cb(error, msg));
+  }
+
+  msg.on('end', onFinish);
+  msg.on('finish', onFinish);
+
+  function onSocket(newSocket) {
+    // remove listener
+    msg.removeListener('socket', onSocket);
+
+    if (finished) return;
+    if (socket) return;
+
+    socket = newSocket;
+
+    // finished on first socket event
+    socket.on('error', onFinish);
+    socket.on('close', onFinish);
+  }
+
+  if (msg.socket) {
+    // socket already assigned
+    onSocket(msg.socket);
+    return;
+  }
+
+  // wait for socket to be assigned
+  msg.on('socket', onSocket);
+};

lib/response.js

@@ -5,19 +5,15 @@
  * Module dependencies.
  */
 
+const ReadStream = require('fs').ReadStream;
 const contentDisposition = require('content-disposition');
-const ensureErrorHandler = require('error-inject');
-const onFinish = require('on-finished');
-const isJSON = require('koa-is-json');
-const escape = require('escape-html');
+const onFinish = require('./onfinish');
+const isJSON = require('./isjson');
 const typeis = require('type-is').is;
 const statuses = require('./statuses');
-const destroy = require('destroy');
 const assert = require('assert');
 const extname = require('path').extname;
-const vary = require('vary');
 const util = require('util');
-const encodeUrl = require('encodeurl');
 
 /**
  * Prototype.
@@ -165,8 +161,25 @@ module.exports = {
 
     // stream
     if ('function' == typeof val.pipe) {
-      onFinish(this.res, destroy.bind(null, val));
-      ensureErrorHandler(val, err => this.ctx.onerror(err));
+      // On finish, destroy the stream
+      onFinish(this.res, () => {
+        // Functionality taken from destroy
+        if (!(val instanceof ReadStream)) {
+          if (typeof val.destroy === 'function') {
+            val.destroy();
+          }
+          return;
+        }
+        if (typeof val.close !== 'function') return;
+
+        // Fix potential bug (?) with node leaving file descriptor open
+        val.on('open', function() {
+          if (typeof this.fd === 'number') {
+            this.close();
+          }
+        });
+      });
+      val.on('error', err => this.ctx.onerror(err));
 
       // overwriting
       if (null != original && original != val) this.remove('Content-Length');
@@ -234,7 +247,14 @@ module.exports = {
   vary(field) {
     if (this.headerSent) return;
 
-    vary(this.res, field);
+    // Revert #291, no reason to include full module
+    // that can be accomplished in 4 extra lines of code
+    let list = this.header.vary;
+    if (!list) return this.set('vary', field);
+
+    list = list.split(/ *, */);
+    if (!~list.indexOf(field)) list.push(field);
+    this.set('vary', list.join(', '));
   },
 
   /**
@@ -259,14 +279,18 @@ module.exports = {
   redirect(url, alt) {
     // location
     if ('back' == url) url = this.ctx.get('Referrer') || alt || '/';
-    this.set('Location', encodeUrl(url));
+    this.set('Location', encodeURI(url));
 
     // status
     if (!statuses.redirect[this.status]) this.status = 302;
 
     // html
     if (this.ctx.headers.accept && this.ctx.headers.accept.indexOf('html') >= 0) {
-      url = escape(url);
+      url = url.replace(/&/g, '&')
+        .replace(/</g, '&lt;')
+        .replace(/>/g, '&gt;')
+        .replace(/"/g, '&quot;')
+        .replace(/'/g, '&#039;');
       this.type = 'text/html; charset=utf-8';
       this.body = `Redirecting to <a href="${url}">${url}</a>.`;
       return;

package.json

@@ -24,17 +24,10 @@
   "dependencies": {
     "content-disposition": "jharrilim/content-disposition#572383f",
     "debug-ms": "~4.1.2",
-    "destroy": "^1.0.4",
-    "encodeurl": "^1.0.2",
-    "error-inject": "^1.0.0",
-    "escape-html": "^1.0.3",
     "fresh": "~0.5.2",
     "http-errors-lite": "^2.0.2",
     "koa-compose": "^4.1.0",
-    "koa-is-json": "^1.0.0",
-    "on-finished": "^2.3.0",
-    "type-is": "^1.6.16",
-    "vary": "^1.1.2"
+    "type-is": "^1.6.16"
   },
   "devDependencies": {
     "egg-bin": "^4.13.0",

test/request/path.js

@@ -3,7 +3,6 @@
 
 const assert = require('assert');
 const context = require('../helpers/context');
-const parseurl = require('parseurl');
 
 describe('ctx.path', () => {
   it('should return the pathname', () => {
@@ -30,11 +29,4 @@ describe('ctx.path=', () => {
     assert.equal(ctx.originalUrl, '/login');
     assert.equal(ctx.request.originalUrl, '/login');
   });
-
-  it('should not affect parseurl', () => {
-    const ctx = context({ url: '/login?foo=bar' });
-    ctx.path = '/login';
-    const url = parseurl(ctx.req);
-    assert.equal(url.path, '/login?foo=bar');
-  });
 });

test/request/querystring.js

@@ -3,7 +3,6 @@
 
 const assert = require('assert');
 const context = require('../helpers/context');
-const parseurl = require('parseurl');
 
 describe('ctx.querystring', () => {
   it('should return the querystring', () => {
@@ -44,11 +43,4 @@ describe('ctx.querystring=', () => {
     assert.equal(ctx.originalUrl, '/store/shoes');
     assert.equal(ctx.request.originalUrl, '/store/shoes');
   });
-
-  it('should not affect parseurl', () => {
-    const ctx = context({ url: '/login?foo=bar' });
-    ctx.querystring = 'foo=bar';
-    const url = parseurl(ctx.req);
-    assert.equal(url.path, '/login?foo=bar');
-  });
 });

test/utils/onfinish.js

@@ -0,0 +1,265 @@
+const assert = require('assert');
+const http = require('http');
+const net = require('net');
+const onFinished = require('../../lib/onfinish');
+
+describe('onFinished(res, listener)', () => {
+  it('should invoke listener given an unknown object', done => {
+    onFinished({}, done);
+  });
+
+  describe('when the response finishes', () => {
+    it('should fire the callback', done => {
+      let server = http.createServer((req, res) => {
+        onFinished(res, done);
+        setTimeout(res.end.bind(res), 0);
+      });
+
+      sendGet(server);
+    });
+
+    it('should include the response object', done => {
+      let server = http.createServer((req, res) => {
+        onFinished(res, (err, msg) => {
+          assert.ok(!err);
+          assert.strictEqual(msg, res);
+          done();
+        });
+        setTimeout(res.end.bind(res), 0);
+      });
+
+      sendGet(server);
+    });
+
+    it('should fire when called after finish', done => {
+      let server = http.createServer((req, res) => {
+        onFinished(res, () => {
+          onFinished(res, done);
+        });
+        setTimeout(res.end.bind(res), 0);
+      });
+
+      sendGet(server);
+    });
+  });
+
+  describe('when using keep-alive', () => {
+    it('should fire for each response', done => {
+      let called = false;
+      let server = http.createServer((req, res) => {
+        onFinished(res, () => {
+          if (called) {
+            socket.end();
+            server.close();
+            done(called !== req ? null : new Error('fired twice on same req'));
+            return;
+          }
+
+          called = req;
+
+          writeRequest(socket);
+        });
+
+        res.end();
+      });
+      let socket;
+
+      server.listen(function(){
+        socket = net.connect(this.address().port, function(){
+          writeRequest(this);
+        });
+      });
+    });
+  });
+
+  describe('when requests pipelined', () => {
+    it('should fire for each request', done => {
+      let count = 0;
+      let responses = [];
+      let server = http.createServer((req, res) => {
+        responses.push(res);
+
+        onFinished(res, err => {
+          assert.ifError(err);
+          assert.strictEqual(responses[0], res);
+          responses.shift();
+
+          if (responses.length === 0) {
+            socket.end();
+            return;
+          }
+
+          responses[0].end('response b');
+        });
+
+        onFinished(req, err => {
+          assert.ifError(err);
+
+          if (++count !== 2) {
+            return;
+          }
+
+          assert.strictEqual(responses.length, 2);
+          responses[0].end('response a');
+        });
+
+        if (responses.length === 1) {
+          // second request
+          writeRequest(socket);
+        }
+
+        req.resume();
+      });
+      let socket;
+
+      server.listen(function(){
+        let data = '';
+        socket = net.connect(this.address().port, function(){
+          writeRequest(this);
+        });
+
+        socket.on('data', chunk => {
+          data += chunk.toString('binary');
+        });
+        socket.on('end', () => {
+          assert.ok(/response a/.test(data));
+          assert.ok(/response b/.test(data));
+          server.close(done);
+        });
+      });
+    });
+  });
+
+  describe('when response errors', () => {
+    it('should fire with error', done => {
+      let server = http.createServer((req, res) => {
+        onFinished(res, err => {
+          assert.ok(err);
+          server.close(done);
+        });
+
+        socket.on('error', noop);
+        socket.write('W');
+      });
+      let socket;
+
+      server.listen(function(){
+        socket = net.connect(this.address().port, function(){
+          writeRequest(this, true);
+        });
+      });
+    });
+
+    it('should include the response object', done => {
+      let server = http.createServer((req, res) => {
+        onFinished(res, (err, msg) => {
+          assert.ok(err);
+          assert.strictEqual(msg, res);
+          server.close(done);
+        });
+
+        socket.on('error', noop);
+        socket.write('W');
+      });
+      let socket;
+
+      server.listen(function(){
+        socket = net.connect(this.address().port, function(){
+          writeRequest(this, true);
+        });
+      });
+    });
+  });
+
+  describe('when the response aborts', () => {
+    it('should execute the callback', done => {
+      let client;
+      let server = http.createServer((req, res) => {
+        onFinished(res, close(server, done));
+        setTimeout(client.abort.bind(client), 0);
+      });
+      server.listen(function(){
+        let port = this.address().port;
+        client = http.get('http://127.0.0.1:' + port);
+        client.on('error', noop);
+      });
+    });
+  });
+
+  describe('when calling many times on same response', () => {
+    it('should not print warnings', done => {
+      let server = http.createServer((req, res) => {
+        let stderr = captureStderr(() => {
+          for (let i = 0; i < 400; i++) {
+            onFinished(res, noop);
+          }
+        });
+
+        onFinished(res, done);
+        assert.strictEqual(stderr, '');
+        res.end();
+      });
+
+      server.listen(function(){
+        let port = this.address().port;
+        http.get('http://127.0.0.1:' + port, res => {
+          res.resume();
+          res.on('end', server.close.bind(server));
+        });
+      });
+    });
+  });
+});
+
+/**********************************************************
+* Removed request tests as those are not needed by our app
+***********************************************************/
+
+function captureStderr(fn){
+  let chunks = [];
+  let write = process.stderr.write;
+
+  process.stderr.write = function write(chunk, encoding){
+    chunks.push(new Buffer(chunk, encoding));
+  };
+
+  try {
+    fn();
+  } finally {
+    process.stderr.write = write;
+  }
+
+  return Buffer.concat(chunks).toString('utf8');
+}
+
+function close(server, callback){
+  return function(error){
+    server.close(err => {
+      callback(error || err);
+    });
+  };
+}
+
+function noop(){}
+
+function sendGet(server){
+  server.listen(function onListening(){
+    let port = this.address().port;
+    http.get('http://127.0.0.1:' + port, res => {
+      res.resume();
+      res.on('end', server.close.bind(server));
+    });
+  });
+}
+
+function writeRequest(socket, chunked){
+  socket.write('GET / HTTP/1.1\r\n');
+  socket.write('Host: localhost\r\n');
+  socket.write('Connection: keep-alive\r\n');
+
+  if (chunked) {
+    socket.write('Transfer-Encoding: chunked\r\n');
+  }
+
+  socket.write('\r\n');
+}