From 8fff8499f1ab57773ff4909c3bd0155ba94ae4f6 Mon Sep 17 00:00:00 2001 From: Anand Suresh Date: Tue, 20 Sep 2016 15:52:52 -0700 Subject: [PATCH 1/6] Fix auto-upgrade from SPDY/3 to SPDY/3.1 The current code auto-upgrades all SPDY/3 clients to SPDY/3.1 when operating in **plain** mode with **autoSpdy31** set to true. This seems rather restrictive, implying that with autoSpdy31 set to true, the server does not support SPDY/3-only clients. This commit fixes this by only upgrading SPDY/3 clients to SPDY/3.1 when the server receives a WINDOW_UPDATE for stream id 0. This ensures that SPDY/3 clients can proceed uninhibited when operating in **plain** mode, and ensuring that the autoSpdy31 option is only used for SPDY/3.1 clients in **plain** mode, allowing for both SPDY/3 and SPDY/3.1 clients to co-exist on the same server in **plain** mode. --- lib/spdy-transport/connection.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/spdy-transport/connection.js b/lib/spdy-transport/connection.js index e3f7db7..7873904 100644 --- a/lib/spdy-transport/connection.js +++ b/lib/spdy-transport/connection.js @@ -240,7 +240,10 @@ Connection.prototype._onVersion = function _onVersion (version) { }) // Update session window - if (state.version >= 3.1 || (state.isServer && state.autoSpdy31)) { this._onSessionWindowDrain() } + if (state.version >= 3.1 || + (state.isServer && state.autoSpdy31 && state.version >= 3.1)) { + this._onSessionWindowDrain() + } this.emit('version', version) } @@ -469,7 +472,8 @@ Connection.prototype._handleHeaders = function _handleHeaders (frame) { Connection.prototype._onSessionWindowDrain = function _onSessionWindowDrain () { var state = this._spdyState - if (state.version < 3.1 && !(state.isServer && state.autoSpdy31)) { + if (state.version < 3.1 && + !(state.isServer && state.autoSpdy31 && state.version >= 3.1)) { return } From 65561f9bb314e21735979c69209541cca5a4f88f Mon Sep 17 00:00:00 2001 From: Anand Suresh Date: Thu, 29 Mar 2018 16:28:45 -0700 Subject: [PATCH 2/6] explicitly set windowBits to its default size (#3) --- lib/spdy-transport/protocol/spdy/zlib-pool.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/spdy-transport/protocol/spdy/zlib-pool.js b/lib/spdy-transport/protocol/spdy/zlib-pool.js index 53c210a..5ac376c 100644 --- a/lib/spdy-transport/protocol/spdy/zlib-pool.js +++ b/lib/spdy-transport/protocol/spdy/zlib-pool.js @@ -24,7 +24,8 @@ function createDeflate (version, compression) { function createInflate (version) { var inflate = zlib.createInflate({ dictionary: transport.protocol.spdy.dictionary[version], - flush: zlib.Z_SYNC_FLUSH + flush: zlib.Z_SYNC_FLUSH, + windowBits: 15 }) // For node.js v0.8 From 0d123946e3009610b67b490542fc87ba516fda10 Mon Sep 17 00:00:00 2001 From: Julian Gautier Date: Wed, 29 Aug 2018 12:48:07 -0700 Subject: [PATCH 3/6] for h2 make sure it follows http messaging from spec --- lib/spdy-transport/stream.js | 48 ++++++++++++++++++++---------- test/both/transport/stream-test.js | 16 +++++++++- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/lib/spdy-transport/stream.js b/lib/spdy-transport/stream.js index f22ea5e..5e2d274 100644 --- a/lib/spdy-transport/stream.js +++ b/lib/spdy-transport/stream.js @@ -25,6 +25,7 @@ function Stream (connection, options) { this.path = options.path this.host = options.host this.headers = options.headers || {} + this.headersSent = false this.connection = connection this.parent = options.parent || null @@ -354,13 +355,20 @@ Stream.prototype._onFinish = function _onFinish () { }) return } - - state.framer.dataFrame({ - id: this.id, - priority: state.priority.getPriority(), - fin: true, - data: new Buffer(0) - }) + if (this.trailers) { + state.framer.headersFrame({ + id: this.id, + headers: this.trailers, + fin: true + }) + } else { + state.framer.dataFrame({ + id: this.id, + priority: state.priority.getPriority(), + fin: true, + data: Buffer.alloc(0) + }) + } } this._maybeClose() @@ -510,6 +518,8 @@ Stream.prototype.send = function send (callback) { this._writableState.finished = true } + this.headersSent = true + // TODO(indunty): ideally it should just take a stream object as an input var self = this this._hardCork() @@ -546,6 +556,9 @@ Stream.prototype.respond = function respond (status, headers, callback) { status: status, headers: headers } + + this.headersSent = true + this._hardCork() state.framer.responseFrame(frame, function (err) { self._hardUncork() @@ -597,14 +610,19 @@ Stream.prototype.sendHeaders = function sendHeaders (headers, callback) { return } - this._hardCork() - state.framer.headersFrame({ - id: this.id, - headers: headers - }, function (err) { - self._hardUncork() - if (callback) { callback(err) } - }) + if (state.protocol.name !== 'h2' || this.headersSent === false) { + this.headersSent = true + this._hardCork() + state.framer.headersFrame({ + id: this.id, + headers: headers + }, function (err) { + self._hardUncork() + if (callback) { callback(err) } + }) + } else { + this.trailers = headers + } } Stream.prototype.destroy = function destroy () { diff --git a/test/both/transport/stream-test.js b/test/both/transport/stream-test.js index 7b608fd..fb19f5a 100644 --- a/test/both/transport/stream-test.js +++ b/test/both/transport/stream-test.js @@ -122,7 +122,6 @@ describe('Transport/Stream', function () { stream.sendHeaders({ a: 'b' }) stream.end() }) - server.on('stream', function (stream) { var gotHeaders = false stream.on('headers', function (headers) { @@ -350,6 +349,7 @@ describe('Transport/Stream', function () { }) it('should emit trailing headers', function (done) { + let dataReceived = false client.request({ method: 'POST', path: '/hello-split' @@ -364,11 +364,25 @@ describe('Transport/Stream', function () { }) }) + if (name === 'h2') { + server.on('frame', function (frame) { + if (frame.type === 'HEADERS' && dataReceived) { + assert.ok(frame.fin) + } + }) + } + server.on('stream', function (stream) { stream.respond(200, {}) stream.resume() + stream.on('data', (data) => { + dataReceived = true + }) stream.on('headers', function (headers) { + if (name === 'h2') { + assert.ok(dataReceived) + } assert.equal(headers.trailer, 'yes') done() }) From b663c2f68a816f40267faaf20b3e340851450e5e Mon Sep 17 00:00:00 2001 From: Julian Gautier Date: Wed, 29 Aug 2018 12:57:52 -0700 Subject: [PATCH 4/6] switch let to var --- test/both/transport/stream-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/both/transport/stream-test.js b/test/both/transport/stream-test.js index fb19f5a..5259a49 100644 --- a/test/both/transport/stream-test.js +++ b/test/both/transport/stream-test.js @@ -349,7 +349,7 @@ describe('Transport/Stream', function () { }) it('should emit trailing headers', function (done) { - let dataReceived = false + var dataReceived = false client.request({ method: 'POST', path: '/hello-split' From 5c335647655430f37f832ebe1d67e10d86a69f4a Mon Sep 17 00:00:00 2001 From: Julian Gautier Date: Wed, 29 Aug 2018 13:01:40 -0700 Subject: [PATCH 5/6] dont use fat arrow --- test/both/transport/stream-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/both/transport/stream-test.js b/test/both/transport/stream-test.js index 5259a49..456dfa3 100644 --- a/test/both/transport/stream-test.js +++ b/test/both/transport/stream-test.js @@ -376,7 +376,7 @@ describe('Transport/Stream', function () { stream.respond(200, {}) stream.resume() - stream.on('data', (data) => { + stream.on('data', function (data) { dataReceived = true }) stream.on('headers', function (headers) { From 50ac1574b00e81daf4f0ff6a43f74e6ea749eb20 Mon Sep 17 00:00:00 2001 From: Julian Gautier Date: Wed, 5 Sep 2018 14:11:55 -0700 Subject: [PATCH 6/6] address comment from Anand --- lib/spdy-transport/stream.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/spdy-transport/stream.js b/lib/spdy-transport/stream.js index 5e2d274..4397b85 100644 --- a/lib/spdy-transport/stream.js +++ b/lib/spdy-transport/stream.js @@ -26,6 +26,7 @@ function Stream (connection, options) { this.host = options.host this.headers = options.headers || {} this.headersSent = false + this.trailers = null this.connection = connection this.parent = options.parent || null