FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: add aligned padding strategy · nodejs/node@0078a97 · GitHub

/ node Public

Commit 0078a97

Browse files
jasnell
authored and
MylesBorins
committed
http2: add aligned padding strategy
Add a padding strategy option that makes a best attempt to ensure that total frame length for DATA and HEADERS frames are aligned on multiples of 8-bytes. Backport-PR-URL: #18050 Backport-PR-URL: #20456 PR-URL: #17938 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 1c313e0 commit 0078a97

4 files changed

Lines changed: 129 additions & 5 deletions

File tree

‎doc/api/http2.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,6 +1671,13 @@ changes:
16711671
* `http2.constants.PADDING_STRATEGY_CALLBACK` - Specifies that the user
16721672
provided `options.selectPadding` callback is to be used to determine the
16731673
amount of padding.
1674+
* `http2.constants.PADDING_STRATEGY_ALIGNED` - Will *attempt* to apply
1675+
enough padding to ensure that the total frame length, including the
1676+
9-byte header, is a multiple of 8. For each frame, however, there is a
1677+
maxmimum allowed number of padding bytes that is determined by current
1678+
flow control state and settings. If this maximum is less than the
1679+
calculated amount needed to ensure alignment, the maximum will be used
1680+
and the total frame length will *not* necessarily be aligned at 8 bytes.
16741681
* `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
16751682
streams for the remote peer as if a SETTINGS frame had been received. Will
16761683
be overridden if the remote peer sets its own value for.
@@ -1742,6 +1749,13 @@ changes:
17421749
* `http2.constants.PADDING_STRATEGY_CALLBACK` - Specifies that the user
17431750
provided `options.selectPadding` callback is to be used to determine the
17441751
amount of padding.
1752+
* `http2.constants.PADDING_STRATEGY_ALIGNED` - Will *attempt* to apply
1753+
enough padding to ensure that the total frame length, including the
1754+
9-byte header, is a multiple of 8. For each frame, however, there is a
1755+
maxmimum allowed number of padding bytes that is determined by current
1756+
flow control state and settings. If this maximum is less than the
1757+
calculated amount needed to ensure alignment, the maximum will be used
1758+
and the total frame length will *not* necessarily be aligned at 8 bytes.
17451759
* `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
17461760
streams for the remote peer as if a SETTINGS frame had been received. Will
17471761
be overridden if the remote peer sets its own value for
@@ -1822,6 +1836,13 @@ changes:
18221836
* `http2.constants.PADDING_STRATEGY_CALLBACK` - Specifies that the user
18231837
provided `options.selectPadding` callback is to be used to determine the
18241838
amount of padding.
1839+
* `http2.constants.PADDING_STRATEGY_ALIGNED` - Will *attempt* to apply
1840+
enough padding to ensure that the total frame length, including the
1841+
9-byte header, is a multiple of 8. For each frame, however, there is a
1842+
maxmimum allowed number of padding bytes that is determined by current
1843+
flow control state and settings. If this maximum is less than the
1844+
calculated amount needed to ensure alignment, the maximum will be used
1845+
and the total frame length will *not* necessarily be aligned at 8 bytes.
18251846
* `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
18261847
streams for the remote peer as if a SETTINGS frame had been received. Will
18271848
be overridden if the remote peer sets its own value for

‎src/node_http2.cc‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,7 @@ Http2Session::Http2Session(Environment* env,
492492
padding_strategy_ = opts.GetPaddingStrategy();
493493

494494
bool hasGetPaddingCallback =
495-
padding_strategy_ == PADDING_STRATEGY_MAX ||
496-
padding_strategy_ == PADDING_STRATEGY_CALLBACK;
495+
padding_strategy_ != PADDING_STRATEGY_NONE;
497496

498497
nghttp2_session_callbacks* callbacks
499498
= callback_struct_saved[hasGetPaddingCallback ? 1 : 0].callbacks;
@@ -682,6 +681,25 @@ inline void Http2Session::RemoveStream(int32_t id) {
682681
streams_.erase(id);
683682
}
684683

684+
// Used as one of the Padding Strategy functions. Will attempt to ensure
685+
// that the total frame size, including header bytes, are 8-byte aligned.
686+
// If maxPayloadLen is smaller than the number of bytes necessary to align,
687+
// will return maxPayloadLen instead.
688+
inline ssize_t Http2Session::OnDWordAlignedPadding(size_t frameLen,
689+
size_t maxPayloadLen) {
690+
size_t r = (frameLen + 9) % 8;
691+
if (r == 0) return frameLen; // If already a multiple of 8, return.
692+
693+
size_t pad = frameLen + (8 - r);
694+
695+
// If maxPayloadLen happens to be less than the calculated pad length,
696+
// use the max instead, even tho this means the frame will not be
697+
// aligned.
698+
pad = std::min(maxPayloadLen, pad);
699+
DEBUG_HTTP2SESSION2(this, "using frame size padding: %d", pad);
700+
return pad;
701+
}
702+
685703
// Used as one of the Padding Strategy functions. Uses the maximum amount
686704
// of padding allowed for the current frame.
687705
inline ssize_t Http2Session::OnMaxFrameSizePadding(size_t frameLen,
@@ -988,9 +1006,21 @@ inline ssize_t Http2Session::OnSelectPadding(nghttp2_session* handle,
9881006
Http2Session* session = static_cast<Http2Session*>(user_data);
9891007
ssize_t padding = frame->hd.length;
9901008

991-
return session->padding_strategy_ == PADDING_STRATEGY_MAX
992-
? session->OnMaxFrameSizePadding(padding, maxPayloadLen)
993-
: session->OnCallbackPadding(padding, maxPayloadLen);
1009+
switch (session->padding_strategy_) {
1010+
case PADDING_STRATEGY_NONE:
1011+
// Fall-through
1012+
break;
1013+
case PADDING_STRATEGY_MAX:
1014+
padding = session->OnMaxFrameSizePadding(padding, maxPayloadLen);
1015+
break;
1016+
case PADDING_STRATEGY_ALIGNED:
1017+
padding = session->OnDWordAlignedPadding(padding, maxPayloadLen);
1018+
break;
1019+
case PADDING_STRATEGY_CALLBACK:
1020+
padding = session->OnCallbackPadding(padding, maxPayloadLen);
1021+
break;
1022+
}
1023+
return padding;
9941024
}
9951025

9961026
#define BAD_PEER_MESSAGE "Remote peer returned unexpected data while we " \
@@ -2873,6 +2903,7 @@ void Initialize(Local<Object> target,
28732903
NODE_DEFINE_CONSTANT(constants, NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE);
28742904

28752905
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_NONE);
2906+
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_ALIGNED);
28762907
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_MAX);
28772908
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_CALLBACK);
28782909

‎src/node_http2.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,8 @@ HTTP_STATUS_CODES(V)
358358
enum padding_strategy_type {
359359
// No padding strategy. This is the default.
360360
PADDING_STRATEGY_NONE,
361+
// Attempts to ensure that the frame is 8-byte aligned
362+
PADDING_STRATEGY_ALIGNED,
361363
// Padding will ensure all data frames are maxFrameSize
362364
PADDING_STRATEGY_MAX,
363365
// Padding will be determined via a JS callback. Note that this can be
@@ -917,6 +919,8 @@ class Http2Session : public AsyncWrap {
917919

918920
private:
919921
// Frame Padding Strategies
922+
inline ssize_t OnDWordAlignedPadding(size_t frameLength,
923+
size_t maxPayloadLen);
920924
inline ssize_t OnMaxFrameSizePadding(size_t frameLength,
921925
size_t maxPayloadLen);
922926
inline ssize_t OnCallbackPadding(size_t frame,

‎test/parallel/test-http2-padding-aligned.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
const { PADDING_STRATEGY_ALIGNED } = http2.constants;
9+
const makeDuplexPair = require('../common/duplexpair');
10+
11+
{
12+
const testData = '<h1>Hello World.</h1>';
13+
const server = http2.createServer({
14+
paddingStrategy: PADDING_STRATEGY_ALIGNED
15+
});
16+
server.on('stream', common.mustCall((stream, headers) => {
17+
stream.respond({
18+
'content-type': 'text/html',
19+
':status': 200
20+
});
21+
stream.end(testData);
22+
}));
23+
24+
const { clientSide, serverSide } = makeDuplexPair();
25+
26+
// The lengths of the expected writes... note that this is highly
27+
// sensitive to how the internals are implemented.
28+
const serverLengths = [24, 9, 9, 32];
29+
const clientLengths = [9, 9, 48, 9, 1, 21, 1, 16];
30+
31+
// Adjust for the 24-byte preamble and two 9-byte settings frames, and
32+
// the result must be equally divisible by 8
33+
assert.strictEqual(
34+
(serverLengths.reduce((i, n) => i + n) - 24 - 9 - 9) % 8, 0);
35+
36+
// Adjust for two 9-byte settings frames, and the result must be equally
37+
// divisible by 8
38+
assert.strictEqual(
39+
(clientLengths.reduce((i, n) => i + n) - 9 - 9) % 8, 0);
40+
41+
serverSide.on('data', common.mustCall((chunk) => {
42+
assert.strictEqual(chunk.length, serverLengths.shift());
43+
}, serverLengths.length));
44+
clientSide.on('data', common.mustCall((chunk) => {
45+
assert.strictEqual(chunk.length, clientLengths.shift());
46+
}, clientLengths.length));
47+
48+
server.emit('connection', serverSide);
49+
50+
const client = http2.connect('http://localhost:80', {
51+
paddingStrategy: PADDING_STRATEGY_ALIGNED,
52+
createConnection: common.mustCall(() => clientSide)
53+
});
54+
55+
const req = client.request({ ':path': '/a' });
56+
57+
req.on('response', common.mustCall());
58+
59+
req.setEncoding('utf8');
60+
req.on('data', common.mustCall((data) => {
61+
assert.strictEqual(data, testData);
62+
}));
63+
req.on('close', common.mustCall(() => {
64+
clientSide.destroy();
65+
clientSide.end();
66+
}));
67+
req.end();
68+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL