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: allow setting the local window size of a session #35978

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/api/errors.md
Expand Up @@ -1226,6 +1226,11 @@ reached.
An attempt was made to initiate a new push stream from within a push stream.
Nested push streams are not permitted.

<a id="ERR_HTTP2_NO_MEM"></a>
### `ERR_HTTP2_NO_MEM`
Copy link
Member

Choose a reason for hiding this comment

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

Not for this PR, but I wonder if it might serve users better to have a single generic memory exhaustion error and combine this and ERR_WORKER_OUT_OF_MEMORY (and prevent a proliferation of other similar errors in the future).

Copy link
Member Author

Choose a reason for hiding this comment

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

a single generic memory exhaustion error

This error is for http2 only and simple enough, maybe it's a little over engineered to do this ?

Copy link
Member

Choose a reason for hiding this comment

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

What's wrong with ENOMEM?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it's more specific to http2.


Out of memory when using the `http2session.setLocalWindowSize(windowSize)` API.

<a id="ERR_HTTP2_NO_SOCKET_MANIPULATION"></a>
### `ERR_HTTP2_NO_SOCKET_MANIPULATION`

Expand Down
23 changes: 23 additions & 0 deletions doc/api/http2.md
Expand Up @@ -519,6 +519,29 @@ added: v8.4.0
A prototype-less object describing the current remote settings of this
`Http2Session`. The remote settings are set by the *connected* HTTP/2 peer.

#### `http2session.setLocalWindowSize(windowSize)`
<!-- YAML
added: REPLACEME
-->

* `windowSize` {number}

Sets the local endpoint's window size.
The `windowSize` is the total window size to set, not
the delta.

```js
const http2 = require('http2');

const server = http2.createServer();
const expectedWindowSize = 2 ** 20;
server.on('connect', (session) => {

// Set local window size to be 2 ** 20
session.setLocalWindowSize(expectedWindowSize);
});
```

#### `http2session.setTimeout(msecs, callback)`
<!-- YAML
added: v8.4.0
Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Expand Up @@ -904,6 +904,7 @@ E('ERR_HTTP2_MAX_PENDING_SETTINGS_ACK',
'Maximum number of pending settings acknowledgements', Error);
E('ERR_HTTP2_NESTED_PUSH',
'A push stream cannot initiate another push stream.', Error);
E('ERR_HTTP2_NO_MEM', 'Out of memory', Error);
E('ERR_HTTP2_NO_SOCKET_MANIPULATION',
'HTTP/2 sockets should not be directly manipulated (e.g. read and written)',
Error);
Expand Down
29 changes: 24 additions & 5 deletions lib/internal/http2/core.js
Expand Up @@ -70,6 +70,7 @@ const {
ERR_HTTP2_INVALID_STREAM,
ERR_HTTP2_MAX_PENDING_SETTINGS_ACK,
ERR_HTTP2_NESTED_PUSH,
ERR_HTTP2_NO_MEM,
ERR_HTTP2_NO_SOCKET_MANIPULATION,
ERR_HTTP2_ORIGIN_LENGTH,
ERR_HTTP2_OUT_OF_STREAMS,
Expand Down Expand Up @@ -101,11 +102,13 @@ const {
},
hideStackFrames
} = require('internal/errors');
const { validateInteger,
validateNumber,
validateString,
validateUint32,
isUint32,
const {
isUint32,
validateInt32,
validateInteger,
validateNumber,
validateString,
validateUint32,
} = require('internal/validators');
const fsPromisesInternal = require('internal/fs/promises');
const { utcDate } = require('internal/http');
Expand Down Expand Up @@ -252,6 +255,7 @@ const {
NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE,
NGHTTP2_ERR_INVALID_ARGUMENT,
NGHTTP2_ERR_STREAM_CLOSED,
NGHTTP2_ERR_NOMEM,

HTTP2_HEADER_AUTHORITY,
HTTP2_HEADER_DATE,
Expand Down Expand Up @@ -1254,6 +1258,21 @@ class Http2Session extends EventEmitter {
this[kHandle].setNextStreamID(id);
}

// Sets the local window size (local endpoints's window size)
// Returns 0 if sucess or throw an exception if NGHTTP2_ERR_NOMEM
// if the window allocation fails
setLocalWindowSize(windowSize) {
if (this.destroyed)
throw new ERR_HTTP2_INVALID_SESSION();

validateInt32(windowSize, 'windowSize', 0);
const ret = this[kHandle].setLocalWindowSize(windowSize);

if (ret === NGHTTP2_ERR_NOMEM) {
this.destroy(new ERR_HTTP2_NO_MEM());
}
}

// If ping is called while we are still connecting, or after close() has
// been called, the ping callback will be invoked immediately will a ping
// cancelled error and a duration of 0.0.
Expand Down
21 changes: 21 additions & 0 deletions src/node_http2.cc
Expand Up @@ -2416,6 +2416,25 @@ void Http2Session::SetNextStreamID(const FunctionCallbackInfo<Value>& args) {
Debug(session, "set next stream id to %d", id);
}

// Set local window size (local endpoints's window size) to the given
// window_size for the stream denoted by 0.
// This function returns 0 if it succeeds, or one of a negative codes
void Http2Session::SetLocalWindowSize(
const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Http2Session* session;
ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder());

int32_t window_size = args[0]->Int32Value(env->context()).ToChecked();

int result = nghttp2_session_set_local_window_size(
session->session(), NGHTTP2_FLAG_NONE, 0, window_size);

args.GetReturnValue().Set(result);

Debug(session, "set local window size to %d", window_size);
}

// A TypedArray instance is shared between C++ and JS land to contain the
// SETTINGS (either remote or local). RefreshSettings updates the current
// values established for each of the settings so those can be read in JS land.
Expand Down Expand Up @@ -3088,6 +3107,8 @@ void Initialize(Local<Object> target,
env->SetProtoMethod(session, "request", Http2Session::Request);
env->SetProtoMethod(session, "setNextStreamID",
Http2Session::SetNextStreamID);
env->SetProtoMethod(session, "setLocalWindowSize",
Http2Session::SetLocalWindowSize);
env->SetProtoMethod(session, "updateChunksSent",
Http2Session::UpdateChunksSent);
env->SetProtoMethod(session, "refreshState", Http2Session::RefreshState);
Expand Down
3 changes: 3 additions & 0 deletions src/node_http2.h
Expand Up @@ -699,6 +699,8 @@ class Http2Session : public AsyncWrap,
static void Settings(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Request(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetNextStreamID(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetLocalWindowSize(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void Goaway(const v8::FunctionCallbackInfo<v8::Value>& args);
static void UpdateChunksSent(const v8::FunctionCallbackInfo<v8::Value>& args);
static void RefreshState(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down Expand Up @@ -1115,6 +1117,7 @@ class Origins {
V(NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE) \
V(NGHTTP2_ERR_INVALID_ARGUMENT) \
V(NGHTTP2_ERR_STREAM_CLOSED) \
V(NGHTTP2_ERR_NOMEM) \
V(STREAM_OPTION_EMPTY_PAYLOAD) \
V(STREAM_OPTION_GET_TRAILERS)

Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-http2-client-destroy.js
Expand Up @@ -77,6 +77,7 @@ const Countdown = require('../common/countdown');
};

assert.throws(() => client.setNextStreamID(), sessionError);
assert.throws(() => client.setLocalWindowSize(), sessionError);
assert.throws(() => client.ping(), sessionError);
assert.throws(() => client.settings({}), sessionError);
assert.throws(() => client.goaway(), sessionError);
Expand All @@ -87,6 +88,7 @@ const Countdown = require('../common/countdown');
// so that state.destroyed is set to true
setImmediate(() => {
assert.throws(() => client.setNextStreamID(), sessionError);
assert.throws(() => client.setLocalWindowSize(), sessionError);
assert.throws(() => client.ping(), sessionError);
assert.throws(() => client.settings({}), sessionError);
assert.throws(() => client.goaway(), sessionError);
Expand Down
121 changes: 121 additions & 0 deletions test/parallel/test-http2-client-setLocalWindowSize.js
@@ -0,0 +1,121 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const http2 = require('http2');

{
const server = http2.createServer();
server.on('stream', common.mustNotCall((stream) => {
stream.respond();
stream.end('ok');
}));

const types = {
boolean: true,
function: () => {},
number: 1,
object: {},
array: [],
null: null,
};

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);

client.on('connect', common.mustCall(() => {
const outOfRangeNum = 2 ** 32;
assert.throws(
() => client.setLocalWindowSize(outOfRangeNum),
{
name: 'RangeError',
code: 'ERR_OUT_OF_RANGE',
message: 'The value of "windowSize" is out of range.' +
' It must be >= 0 && <= 2147483647. Received ' + outOfRangeNum
}
);

// Throw if something other than number is passed to setLocalWindowSize
Object.entries(types).forEach(([type, value]) => {
if (type === 'number') {
return;
}

assert.throws(
() => client.setLocalWindowSize(value),
{
name: 'TypeError',
code: 'ERR_INVALID_ARG_TYPE',
message: 'The "windowSize" argument must be of type number.' +
common.invalidArgTypeHelper(value)
}
);
});

server.close();
client.close();
}));
}));
}

{
const server = http2.createServer();
server.on('stream', common.mustNotCall((stream) => {
stream.respond();
stream.end('ok');
}));

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);

client.on('connect', common.mustCall(() => {
const windowSize = 2 ** 20;
const defaultSetting = http2.getDefaultSettings();
client.setLocalWindowSize(windowSize);

assert.strictEqual(client.state.effectiveLocalWindowSize, windowSize);
assert.strictEqual(client.state.localWindowSize, windowSize);
assert.strictEqual(
client.state.remoteWindowSize,
defaultSetting.initialWindowSize
);

server.close();
client.close();
}));
}));
}

{
const server = http2.createServer();
server.on('stream', common.mustNotCall((stream) => {
stream.respond();
stream.end('ok');
}));

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);

client.on('connect', common.mustCall(() => {
const windowSize = 20;
const defaultSetting = http2.getDefaultSettings();
client.setLocalWindowSize(windowSize);

assert.strictEqual(client.state.effectiveLocalWindowSize, windowSize);
assert.strictEqual(
client.state.localWindowSize,
defaultSetting.initialWindowSize
);
assert.strictEqual(
client.state.remoteWindowSize,
defaultSetting.initialWindowSize
);

server.close();
client.close();
}));
}));
}
37 changes: 37 additions & 0 deletions test/parallel/test-http2-server-setLocalWindowSize.js
@@ -0,0 +1,37 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const http2 = require('http2');

const server = http2.createServer();
server.on('stream', common.mustCall((stream) => {
stream.respond();
stream.end('ok');
}));
server.on('session', common.mustCall((session) => {
const windowSize = 2 ** 20;
const defaultSetting = http2.getDefaultSettings();
session.setLocalWindowSize(windowSize);

assert.strictEqual(session.state.effectiveLocalWindowSize, windowSize);
assert.strictEqual(session.state.localWindowSize, windowSize);
assert.strictEqual(
session.state.remoteWindowSize,
defaultSetting.initialWindowSize
);
}));

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);

const req = client.request();
req.resume();
req.on('close', common.mustCall(() => {
client.close();
server.close();
}));
}));