diff options
author | James M Snell <jasnell@gmail.com> | 2017-10-16 20:43:54 +0300 |
---|---|---|
committer | Myles Borins <mylesborins@google.com> | 2017-10-24 00:35:54 +0300 |
commit | 0d50c7061c2c86c1eaa48979c4b0bb14b2f30a84 (patch) | |
tree | 6c54e50455d557dbcf24e3259a9e269db6d86e81 /test | |
parent | 5d34f2f5a762698081f49ab97d167e05c74dcf53 (diff) |
http2: multiple style and performance updates
* move CHECK statements into DEBUG checks
* improve performance by removing branches
* Several if checks were left in while the code was being developed.
Now that the core API has stablized more, the checks are largely
unnecessary and can be removed, yielding a significant boost in
performance.
* refactor flow control for proper backpressure
* use std::queue for inbound headers
* use std::queue for outbound data
* remove now unnecessary FreeHeaders function
* expand comments and miscellaneous edits
* add a couple of misbehaving flow control tests
PR-URL: https://github.com/nodejs/node/pull/16239
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Diffstat (limited to 'test')
-rw-r--r-- | test/parallel/test-http2-misbehaving-flow-control-paused.js | 89 | ||||
-rw-r--r-- | test/parallel/test-http2-misbehaving-flow-control.js | 81 |
2 files changed, 170 insertions, 0 deletions
diff --git a/test/parallel/test-http2-misbehaving-flow-control-paused.js b/test/parallel/test-http2-misbehaving-flow-control-paused.js new file mode 100644 index 00000000000..ee799b1d5a2 --- /dev/null +++ b/test/parallel/test-http2-misbehaving-flow-control-paused.js @@ -0,0 +1,89 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const h2 = require('http2'); +const net = require('net'); + +const preamble = Buffer.from([ + 0x50, 0x52, 0x49, 0x20, 0x2a, 0x20, 0x48, 0x54, 0x54, 0x50, 0x2f, + 0x32, 0x2e, 0x30, 0x0d, 0x0a, 0x0d, 0x0a, 0x53, 0x4d, 0x0d, 0x0a, + 0x0d, 0x0a, 0x00, 0x00, 0x0c, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x03, 0x00, 0x00, 0x00, 0x64, 0x00, 0x04, 0x00, 0x00, 0xff, + 0xff, 0x00, 0x00, 0x00, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, + 0x00, 0xc8, 0x00, 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00, 0x05, + 0x00, 0x00, 0x00, 0x00, 0x64, 0x00, 0x00, 0x05, 0x02, 0x00, 0x00, + 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, + 0x02, 0x00, 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x07, 0x00, + 0x00, 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x00, + 0x00, 0x03, 0x00, 0x00, 0x00, 0x2c, 0x01, 0x24, 0x00, 0x00, 0x00, + 0x0d, 0x00, 0x00, 0x00, 0x0b, 0x0f, 0x83, 0x84, 0x86, 0x41, 0x8a, + 0xa0, 0xe4, 0x1d, 0x13, 0x9d, 0x09, 0xb8, 0xf0, 0x1e, 0x07, 0x53, + 0x03, 0x2a, 0x2f, 0x2a, 0x90, 0x7a, 0x8a, 0xaa, 0x69, 0xd2, 0x9a, + 0xc4, 0xc0, 0x57, 0x0b, 0xcb, 0x87, 0x0f, 0x0d, 0x83, 0x08, 0x00, + 0x0f +]); + +const data = Buffer.from([ + 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0d, + 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x0a, 0x68, 0x65, 0x6c, + 0x6c, 0x6f, 0x0a, 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x0a +]); + +// This is testing the case of a misbehaving client that is not paying +// attention to flow control. The initial window size is set to data +// payload * 2, which in this case is 36, the stream is paused so +// WINDOW_UPDATE frames are not being sent, which means the window +// size is not being updated. A well behaved client is supposed to +// stop sending until the window size is expanded again. +// +// However, our malicious client keeps sending data beyond the flow +// control window! +// +// Bad client! Bad! +// +// Fortunately, nghttp2 handles this situation for us by keeping track +// of the flow control window and responding with a FLOW_CONTROL_ERROR +// causing the stream to get shut down... +// +// At least, that's what is supposed to happen. + +let client; + +const server = h2.createServer({ settings: { initialWindowSize: 36 } }); +server.on('stream', (stream) => { + + // Not reading causes the flow control window to get backed up. + stream.pause(); + + stream.on('error', common.mustCall((err) => { + common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 3' + })(err); + server.close(); + client.destroy(); + })); + + stream.on('end', common.mustNotCall()); + + stream.respond(); + stream.end('ok'); +}); + +server.listen(0, () => { + client = net.connect(server.address().port, () => { + client.on('error', console.log); + + client.write(preamble); + + client.write(data); + client.write(data); + client.write(data); + }); +}); diff --git a/test/parallel/test-http2-misbehaving-flow-control.js b/test/parallel/test-http2-misbehaving-flow-control.js new file mode 100644 index 00000000000..010e0774131 --- /dev/null +++ b/test/parallel/test-http2-misbehaving-flow-control.js @@ -0,0 +1,81 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const h2 = require('http2'); +const net = require('net'); + +const preamble = Buffer.from([ + 0x50, 0x52, 0x49, 0x20, 0x2a, 0x20, 0x48, 0x54, 0x54, 0x50, 0x2f, + 0x32, 0x2e, 0x30, 0x0d, 0x0a, 0x0d, 0x0a, 0x53, 0x4d, 0x0d, 0x0a, + 0x0d, 0x0a, 0x00, 0x00, 0x0c, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x03, 0x00, 0x00, 0x00, 0x64, 0x00, 0x04, 0x00, 0x00, 0xff, + 0xff, 0x00, 0x00, 0x00, 0x04, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, + 0x00, 0xc8, 0x00, 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00, 0x05, + 0x00, 0x00, 0x00, 0x00, 0x64, 0x00, 0x00, 0x05, 0x02, 0x00, 0x00, + 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, + 0x02, 0x00, 0x00, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, 0x07, 0x00, + 0x00, 0x00, 0x05, 0x02, 0x00, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x00, + 0x00, 0x03, 0x00, 0x00, 0x00, 0x2c, 0x01, 0x24, 0x00, 0x00, 0x00, + 0x0d, 0x00, 0x00, 0x00, 0x0b, 0x0f, 0x83, 0x84, 0x86, 0x41, 0x8a, + 0xa0, 0xe4, 0x1d, 0x13, 0x9d, 0x09, 0xb8, 0xf0, 0x1e, 0x07, 0x53, + 0x03, 0x2a, 0x2f, 0x2a, 0x90, 0x7a, 0x8a, 0xaa, 0x69, 0xd2, 0x9a, + 0xc4, 0xc0, 0x57, 0x0b, 0xcb, 0x87, 0x0f, 0x0d, 0x83, 0x08, 0x00, + 0x0f +]); + +const data = Buffer.from([ + 0x00, 0x00, 0x12, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0d, + 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x0a, 0x68, 0x65, 0x6c, + 0x6c, 0x6f, 0x0a, 0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x0a +]); + +// This is testing the case of a misbehaving client that is not paying +// attention to flow control. The initial window size is set to data +// payload, which in this case is 18, the stream is set to flowing so +// WINDOW_UPDATE frames are being sent, but the client is writing +// faster than those can be read. +// +// Bad client! Bad! +// +// Fortunately, nghttp2 handles this situation for us by keeping track +// of the flow control window and responding with a FLOW_CONTROL_ERROR +// causing the stream to get shut down... +// +// At least, that's what is supposed to happen. + +let client; +const server = h2.createServer({ settings: { initialWindowSize: 18 } }); +server.on('stream', (stream) => { + + stream.resume(); + + stream.on('error', common.mustCall((err) => { + common.expectsError({ + code: 'ERR_HTTP2_STREAM_ERROR', + type: Error, + message: 'Stream closed with error code 3' + })(err); + server.close(); + client.destroy(); + })); + + stream.respond(); + stream.end('ok'); +}); + +server.listen(0, () => { + client = net.connect(server.address().port, () => { + client.on('error', console.log); + + client.write(preamble); + + client.write(data); + client.write(data); + client.write(data); + }); +}); |