Skip to content

Commit

Permalink
http2: add aligned padding strategy
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
jasnell authored and MylesBorins committed May 2, 2018
1 parent 1c313e0 commit 0078a97
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 5 deletions.
21 changes: 21 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -1671,6 +1671,13 @@ changes:
* `http2.constants.PADDING_STRATEGY_CALLBACK` - Specifies that the user
provided `options.selectPadding` callback is to be used to determine the
amount of padding.
* `http2.constants.PADDING_STRATEGY_ALIGNED` - Will *attempt* to apply
enough padding to ensure that the total frame length, including the
9-byte header, is a multiple of 8. For each frame, however, there is a
maxmimum allowed number of padding bytes that is determined by current
flow control state and settings. If this maximum is less than the
calculated amount needed to ensure alignment, the maximum will be used
and the total frame length will *not* necessarily be aligned at 8 bytes.
* `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
streams for the remote peer as if a SETTINGS frame had been received. Will
be overridden if the remote peer sets its own value for.
Expand Down Expand Up @@ -1742,6 +1749,13 @@ changes:
* `http2.constants.PADDING_STRATEGY_CALLBACK` - Specifies that the user
provided `options.selectPadding` callback is to be used to determine the
amount of padding.
* `http2.constants.PADDING_STRATEGY_ALIGNED` - Will *attempt* to apply
enough padding to ensure that the total frame length, including the
9-byte header, is a multiple of 8. For each frame, however, there is a
maxmimum allowed number of padding bytes that is determined by current
flow control state and settings. If this maximum is less than the
calculated amount needed to ensure alignment, the maximum will be used
and the total frame length will *not* necessarily be aligned at 8 bytes.
* `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
streams for the remote peer as if a SETTINGS frame had been received. Will
be overridden if the remote peer sets its own value for
Expand Down Expand Up @@ -1822,6 +1836,13 @@ changes:
* `http2.constants.PADDING_STRATEGY_CALLBACK` - Specifies that the user
provided `options.selectPadding` callback is to be used to determine the
amount of padding.
* `http2.constants.PADDING_STRATEGY_ALIGNED` - Will *attempt* to apply
enough padding to ensure that the total frame length, including the
9-byte header, is a multiple of 8. For each frame, however, there is a
maxmimum allowed number of padding bytes that is determined by current
flow control state and settings. If this maximum is less than the
calculated amount needed to ensure alignment, the maximum will be used
and the total frame length will *not* necessarily be aligned at 8 bytes.
* `peerMaxConcurrentStreams` {number} Sets the maximum number of concurrent
streams for the remote peer as if a SETTINGS frame had been received. Will
be overridden if the remote peer sets its own value for
Expand Down
41 changes: 36 additions & 5 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,7 @@ Http2Session::Http2Session(Environment* env,
padding_strategy_ = opts.GetPaddingStrategy();

bool hasGetPaddingCallback =
padding_strategy_ == PADDING_STRATEGY_MAX ||
padding_strategy_ == PADDING_STRATEGY_CALLBACK;
padding_strategy_ != PADDING_STRATEGY_NONE;

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

// Used as one of the Padding Strategy functions. Will attempt to ensure
// that the total frame size, including header bytes, are 8-byte aligned.
// If maxPayloadLen is smaller than the number of bytes necessary to align,
// will return maxPayloadLen instead.
inline ssize_t Http2Session::OnDWordAlignedPadding(size_t frameLen,
size_t maxPayloadLen) {
size_t r = (frameLen + 9) % 8;
if (r == 0) return frameLen; // If already a multiple of 8, return.

size_t pad = frameLen + (8 - r);

// If maxPayloadLen happens to be less than the calculated pad length,
// use the max instead, even tho this means the frame will not be
// aligned.
pad = std::min(maxPayloadLen, pad);
DEBUG_HTTP2SESSION2(this, "using frame size padding: %d", pad);
return pad;
}

// Used as one of the Padding Strategy functions. Uses the maximum amount
// of padding allowed for the current frame.
inline ssize_t Http2Session::OnMaxFrameSizePadding(size_t frameLen,
Expand Down Expand Up @@ -988,9 +1006,21 @@ inline ssize_t Http2Session::OnSelectPadding(nghttp2_session* handle,
Http2Session* session = static_cast<Http2Session*>(user_data);
ssize_t padding = frame->hd.length;

return session->padding_strategy_ == PADDING_STRATEGY_MAX
? session->OnMaxFrameSizePadding(padding, maxPayloadLen)
: session->OnCallbackPadding(padding, maxPayloadLen);
switch (session->padding_strategy_) {
case PADDING_STRATEGY_NONE:
// Fall-through
break;
case PADDING_STRATEGY_MAX:
padding = session->OnMaxFrameSizePadding(padding, maxPayloadLen);
break;
case PADDING_STRATEGY_ALIGNED:
padding = session->OnDWordAlignedPadding(padding, maxPayloadLen);
break;
case PADDING_STRATEGY_CALLBACK:
padding = session->OnCallbackPadding(padding, maxPayloadLen);
break;
}
return padding;
}

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

NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_NONE);
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_ALIGNED);
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_MAX);
NODE_DEFINE_CONSTANT(constants, PADDING_STRATEGY_CALLBACK);

Expand Down
4 changes: 4 additions & 0 deletions src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,8 @@ HTTP_STATUS_CODES(V)
enum padding_strategy_type {
// No padding strategy. This is the default.
PADDING_STRATEGY_NONE,
// Attempts to ensure that the frame is 8-byte aligned
PADDING_STRATEGY_ALIGNED,
// Padding will ensure all data frames are maxFrameSize
PADDING_STRATEGY_MAX,
// Padding will be determined via a JS callback. Note that this can be
Expand Down Expand Up @@ -917,6 +919,8 @@ class Http2Session : public AsyncWrap {

private:
// Frame Padding Strategies
inline ssize_t OnDWordAlignedPadding(size_t frameLength,
size_t maxPayloadLen);
inline ssize_t OnMaxFrameSizePadding(size_t frameLength,
size_t maxPayloadLen);
inline ssize_t OnCallbackPadding(size_t frame,
Expand Down
68 changes: 68 additions & 0 deletions test/parallel/test-http2-padding-aligned.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const http2 = require('http2');
const { PADDING_STRATEGY_ALIGNED } = http2.constants;
const makeDuplexPair = require('../common/duplexpair');

{
const testData = '<h1>Hello World.</h1>';
const server = http2.createServer({
paddingStrategy: PADDING_STRATEGY_ALIGNED
});
server.on('stream', common.mustCall((stream, headers) => {
stream.respond({
'content-type': 'text/html',
':status': 200
});
stream.end(testData);
}));

const { clientSide, serverSide } = makeDuplexPair();

// The lengths of the expected writes... note that this is highly
// sensitive to how the internals are implemented.
const serverLengths = [24, 9, 9, 32];
const clientLengths = [9, 9, 48, 9, 1, 21, 1, 16];

// Adjust for the 24-byte preamble and two 9-byte settings frames, and
// the result must be equally divisible by 8
assert.strictEqual(
(serverLengths.reduce((i, n) => i + n) - 24 - 9 - 9) % 8, 0);

// Adjust for two 9-byte settings frames, and the result must be equally
// divisible by 8
assert.strictEqual(
(clientLengths.reduce((i, n) => i + n) - 9 - 9) % 8, 0);

serverSide.on('data', common.mustCall((chunk) => {
assert.strictEqual(chunk.length, serverLengths.shift());
}, serverLengths.length));
clientSide.on('data', common.mustCall((chunk) => {
assert.strictEqual(chunk.length, clientLengths.shift());
}, clientLengths.length));

server.emit('connection', serverSide);

const client = http2.connect('http://localhost:80', {
paddingStrategy: PADDING_STRATEGY_ALIGNED,
createConnection: common.mustCall(() => clientSide)
});

const req = client.request({ ':path': '/a' });

req.on('response', common.mustCall());

req.setEncoding('utf8');
req.on('data', common.mustCall((data) => {
assert.strictEqual(data, testData);
}));
req.on('close', common.mustCall(() => {
clientSide.destroy();
clientSide.end();
}));
req.end();
}

0 comments on commit 0078a97

Please sign in to comment.