Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http2: add aligned padding strategy #17938

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nit: maxmimum -> maximum.

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 @@ -1596,6 +1603,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 @@ -1676,6 +1690,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 @@ -484,8 +484,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 @@ -579,6 +578,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 @@ -873,9 +891,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 @@ -2681,6 +2711,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 @@ -356,6 +356,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 @@ -882,6 +884,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();
}