From d655f208cb60dfa733ac02ffedbbde8f52e92762 Mon Sep 17 00:00:00 2001 From: Jonatan Nilsson Date: Sat, 12 Oct 2019 00:17:22 +0000 Subject: [PATCH] Remove destroy, encoder, error-inject, escape-html, koa-is-json, on-finished, type-is and vary --- lib/application.js | 4 +- lib/isjson.js | 13 ++ lib/onfinish.js | 74 +++++++ lib/response.js | 48 +++-- package.json | 9 +- test/request/path.js | 8 - test/request/querystring.js | 8 - test/{parseurl => utils}/fastparse.js | 0 test/utils/onfinish.js | 265 ++++++++++++++++++++++++++ 9 files changed, 391 insertions(+), 38 deletions(-) create mode 100644 lib/isjson.js create mode 100644 lib/onfinish.js rename test/{parseurl => utils}/fastparse.js (100%) create mode 100644 test/utils/onfinish.js diff --git a/lib/application.js b/lib/application.js index 02a6604..e931a30 100644 --- a/lib/application.js +++ b/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'); diff --git a/lib/isjson.js b/lib/isjson.js new file mode 100644 index 0000000..2ad4473 --- /dev/null +++ b/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; diff --git a/lib/onfinish.js b/lib/onfinish.js new file mode 100644 index 0000000..e0cf0d7 --- /dev/null +++ b/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); +}; diff --git a/lib/response.js b/lib/response.js index 99ebd7c..7195246 100644 --- a/lib/response.js +++ b/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, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); this.type = 'text/html; charset=utf-8'; this.body = `Redirecting to ${url}.`; return; diff --git a/package.json b/package.json index c4ab5b3..bfe7ea0 100644 --- a/package.json +++ b/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", diff --git a/test/request/path.js b/test/request/path.js index e432a30..2186ca3 100644 --- a/test/request/path.js +++ b/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'); - }); }); diff --git a/test/request/querystring.js b/test/request/querystring.js index 13287c1..7b7c2bd 100644 --- a/test/request/querystring.js +++ b/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'); - }); }); diff --git a/test/parseurl/fastparse.js b/test/utils/fastparse.js similarity index 100% rename from test/parseurl/fastparse.js rename to test/utils/fastparse.js diff --git a/test/utils/onfinish.js b/test/utils/onfinish.js new file mode 100644 index 0000000..86ba729 --- /dev/null +++ b/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'); +}