From 51dfdc2758f9174c0a0dd41ad798c84782fa95f5 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 12 May 2020 10:56:22 +0200 Subject: [PATCH 1/5] refactor: cleanup --- lib/client.js | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/client.js b/lib/client.js index f1900ff3dd5..0089c7253b9 100644 --- a/lib/client.js +++ b/lib/client.js @@ -36,7 +36,6 @@ class Parser extends HTTPParser { this.socket = socket this.body = null this.read = 0 - this.callback = null } [HTTPParser.kOnHeaders] () { @@ -151,7 +150,8 @@ class Parser extends HTTPParser { } } client[kQueue].splice(0, client[kInflight]) - client[kInflight] = client[kComplete] = 0 + client[kInflight] = 0 + client[kComplete] = 0 while (retry.length) { client[kQueue].unshift(retry.pop()) @@ -310,32 +310,32 @@ function resume (client) { client[kInflight]++ - client._socket.cork() - client._socket.write(`${method} ${path} HTTP/1.1\r\nConnection: keep-alive\r\n`, 'ascii') + const socket = client._socket + + socket.cork() + socket.write(`${method} ${path} HTTP/1.1\r\nConnection: keep-alive\r\n`, 'ascii') if (!host) { - client._socket.write('Host: ' + client[kUrl].hostname + '\r\n', 'ascii') + socket.write('Host: ' + client[kUrl].hostname + '\r\n', 'ascii') } - client._socket.write(rawHeaders, 'ascii') + socket.write(rawHeaders, 'ascii') if (typeof body === 'string' || body instanceof Uint8Array) { if (chunked) { - client._socket.write(`content-length: ${Buffer.byteLength(body)}\r\n\r\n`, 'ascii') + socket.write(`content-length: ${Buffer.byteLength(body)}\r\n\r\n`, 'ascii') } else { - client._socket.write('\r\n') + socket.write('\r\n') } - client._socket.write(body) + socket.write(body) endRequest(client) } else if (body && typeof body.pipe === 'function') { if (chunked) { - client._socket.write('transfer-encoding: chunked\r\n', 'ascii') + socket.write('transfer-encoding: chunked\r\n', 'ascii') } else { - client._socket.write('\r\n', 'ascii') + socket.write('\r\n', 'ascii') } let finished = false - const socket = client._socket - const onData = (chunk) => { if (chunked) { socket.write('\r\n' + Buffer.byteLength(chunk).toString(16) + '\r\n') @@ -396,6 +396,7 @@ function resume (client) { endRequest(client) } } + class Client extends EventEmitter { constructor (url, opts = {}) { super() From cda368ac03cc96d992ba75e4ad0b542ccf682739 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 12 May 2020 10:58:23 +0200 Subject: [PATCH 2/5] fix: don't resume while piping request --- lib/client.js | 12 +++++++++ test/client-pipelining.js | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/lib/client.js b/lib/client.js index 0089c7253b9..b63481ec958 100644 --- a/lib/client.js +++ b/lib/client.js @@ -12,6 +12,7 @@ const assert = require('assert') const stream = require('stream') const kUrl = Symbol('url') +const kWriting = Symbol('writing') const kQueue = Symbol('queue') const kTimeout = Symbol('timeout') const kTLSOpts = Symbol('TLS Options') @@ -243,6 +244,8 @@ function connect (client) { const endRequest = (client) => { client._socket.write('\r\n', 'ascii') client._socket.uncork() + + client[kWriting] = false resume(client) } @@ -279,6 +282,10 @@ function resume (client) { return } + if (client[kWriting]) { + return + } + const { host, method, @@ -309,6 +316,7 @@ function resume (client) { } client[kInflight]++ + client[kWriting] = true const socket = client._socket @@ -370,6 +378,9 @@ function resume (client) { if (!socket.destroyed) { socket.destroy(err) } + + client[kWriting] = false + resume(client) } else { if (chunked) { socket.cork() @@ -430,6 +441,7 @@ class Client extends EventEmitter { this[kRetryDelay] = 0 this[kRetryTimeout] = null this[kOnDestroyed] = [] + this[kWriting] = false this[kQueue] = [] this[kInflight] = 0 this[kComplete] = 0 diff --git a/test/client-pipelining.js b/test/client-pipelining.js index 68fe36fe8ed..8164c3cecf6 100644 --- a/test/client-pipelining.js +++ b/test/client-pipelining.js @@ -372,3 +372,54 @@ test('pipelining non-idempotent', (t) => { }) }) }) + +test('pipelining non-idempotent w body', (t) => { + t.plan(4) + + const server = createServer() + server.on('request', (req, res) => { + setImmediate(() => { + res.end('asd') + }) + }) + t.tearDown(server.close.bind(server)) + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`, { + pipelining: 2 + }) + t.tearDown(client.close.bind(client)) + + let ended = false + client.request({ + path: '/', + method: 'POST', + body: new Readable({ + read () { + this.push('asd') + setImmediate(() => { + this.push(null) + ended = true + }) + } + }) + }, (err, data) => { + t.error(err) + data.body + .resume() + .on('end', () => { + t.pass() + }) + }) + + client.request({ + path: '/', + method: 'GET', + idempotent: false + }, (err, data) => { + t.error(err) + t.strictEqual(ended, true) + data.body.resume() + }) + }) +}) From cb7b5bbad40c90dcf2d9b467208452491610c18d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 12 May 2020 13:11:47 +0200 Subject: [PATCH 3/5] refactor: splice in retry --- lib/client.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/client.js b/lib/client.js index b63481ec958..7928530535a 100644 --- a/lib/client.js +++ b/lib/client.js @@ -150,14 +150,10 @@ class Parser extends HTTPParser { process.nextTick(callback, err, null) } } - client[kQueue].splice(0, client[kInflight]) + client[kQueue].splice(0, client[kInflight], ...retry) client[kInflight] = 0 client[kComplete] = 0 - while (retry.length) { - client[kQueue].unshift(retry.pop()) - } - resume(client) } } From 3d6f8bcbc7dbfd3ed313e17241913833579c62da Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 12 May 2020 13:17:58 +0200 Subject: [PATCH 4/5] fix: remove unnecessary eslint pragma --- lib/client.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/client.js b/lib/client.js index 7928530535a..595eeecca9c 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1,7 +1,5 @@ 'use strict' -/* eslint no-prototype-builtins: "off" */ - const { URL } = require('url') const net = require('net') const tls = require('tls') From 33bca9a10e94c43eb55bcb4308505538e8d1467a Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 12 May 2020 13:20:44 +0200 Subject: [PATCH 5/5] refactor: explicitly check pending --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 595eeecca9c..98b9f251112 100644 --- a/lib/client.js +++ b/lib/client.js @@ -214,7 +214,7 @@ function _connect (client) { client._socket = null client._parser = null - if (client[kQueue].length > 0) { + if (client.pending > 0) { connect(client) }