From 50d3088dc30d4cea8f746e503cf2b50c900a6acc Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Tue, 24 Oct 2017 23:56:56 +0800 Subject: [PATCH 1/2] http2: custom priority for push streams Sets the priority silently on push streams. Previously the weight options, whilst documented, was simply ignored. Note that the client is not informed of the non-default priority. The nghttp2 library ignores push stream flags (e.g. 0x20 for priority). It also currently offers no way to set priority value during push stream creation. This appears to be by design, as discussed here: - https://github.com/nghttp2/nghttp2/issues/429 - https://github.com/nghttp2/nghttp2/pull/430 --- lib/internal/http2/core.js | 7 +++ .../test-http2-server-push-priority.js | 46 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 test/parallel/test-http2-server-push-priority.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 8d8f200262d1d9..9bd503a799e4f5 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1804,6 +1804,13 @@ class ServerHttp2Stream extends Http2Stream { const stream = new ServerHttp2Stream(session, ret, options, headers); + if (options.weight !== undefined) { + stream.priority({ + weight: options.weight, + silent: true + }); + } + // If the push stream is a head request, close the writable side of // the stream immediately as there won't be any data sent. if (headRequest) { diff --git a/test/parallel/test-http2-server-push-priority.js b/test/parallel/test-http2-server-push-priority.js new file mode 100644 index 00000000000000..236969ae0bbfdf --- /dev/null +++ b/test/parallel/test-http2-server-push-priority.js @@ -0,0 +1,46 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) +common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); + +const expectedWeight = 256; + +const server = http2.createServer(); +server.on('stream', common.mustCall((stream, headers, flags) => { + const options = { weight: expectedWeight }; + stream.pushStream({}, options, common.mustCall((stream, headers, flags) => { + // The priority of push streams is changed silently by nghttp2, + // without emitting a PRIORITY frame. Flags are ignored. + assert.strictEqual(stream.state.weight, expectedWeight); + // assert.ok(flags & http2.constants.NGHTTP2_FLAG_PRIORITY); + })); + stream.respond({ + 'content-type': 'text/html', + ':status': 200 + }); + stream.end('test'); +})); + +server.listen(0, common.mustCall(() => { + const port = server.address().port; + const client = http2.connect(`http://localhost:${port}`); + + const headers = { ':path': '/' }; + const req = client.request(headers); + + client.on('stream', common.mustCall((stream, headers, flags) => { + // Since the push priority is set silently, the client is not informed. + // assert.strictEqual(stream.state.weight, expectedWeight); + // assert.ok(flags & http2.constants.NGHTTP2_FLAG_PRIORITY); + })); + + req.on('data', common.mustCall(() => {})); + req.on('end', common.mustCall(() => { + server.close(); + client.destroy(); + })); + req.end(); +})); From d5f100dbc21b9478b10b244a3eb0545130ff6b86 Mon Sep 17 00:00:00 2001 From: Sebastiaan Deckers Date: Wed, 25 Oct 2017 02:12:59 +0800 Subject: [PATCH 2/2] explicit checking of silent priority (squash) --- test/parallel/test-http2-server-push-priority.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-http2-server-push-priority.js b/test/parallel/test-http2-server-push-priority.js index 236969ae0bbfdf..e9753ccec48593 100644 --- a/test/parallel/test-http2-server-push-priority.js +++ b/test/parallel/test-http2-server-push-priority.js @@ -2,20 +2,20 @@ const common = require('../common'); if (!common.hasCrypto) -common.skip('missing crypto'); + common.skip('missing crypto'); const assert = require('assert'); const http2 = require('http2'); -const expectedWeight = 256; const server = http2.createServer(); server.on('stream', common.mustCall((stream, headers, flags) => { + const expectedWeight = 256; const options = { weight: expectedWeight }; stream.pushStream({}, options, common.mustCall((stream, headers, flags) => { // The priority of push streams is changed silently by nghttp2, // without emitting a PRIORITY frame. Flags are ignored. assert.strictEqual(stream.state.weight, expectedWeight); - // assert.ok(flags & http2.constants.NGHTTP2_FLAG_PRIORITY); + assert.strictEqual(flags & http2.constants.NGHTTP2_FLAG_PRIORITY, 0); })); stream.respond({ 'content-type': 'text/html', @@ -33,8 +33,9 @@ server.listen(0, common.mustCall(() => { client.on('stream', common.mustCall((stream, headers, flags) => { // Since the push priority is set silently, the client is not informed. - // assert.strictEqual(stream.state.weight, expectedWeight); - // assert.ok(flags & http2.constants.NGHTTP2_FLAG_PRIORITY); + const expectedWeight = http2.constants.NGHTTP2_DEFAULT_WEIGHT; + assert.strictEqual(stream.state.weight, expectedWeight); + assert.strictEqual(flags & http2.constants.NGHTTP2_FLAG_PRIORITY, 0); })); req.on('data', common.mustCall(() => {}));