Skip to content

Commit

Permalink
http2: add ping event
Browse files Browse the repository at this point in the history
Add a `Http2Session` event whenever a non-ack `PING` is received.

Fixes: #18514

Backport-PR-URL: #22850
PR-URL: #23009
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Denys Otrishko <[email protected]>
  • Loading branch information
jasnell authored and BethGriggs committed Oct 16, 2018
1 parent ca933ce commit 10576d6
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 9 deletions.
10 changes: 10 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ session.on('localSettings', (settings) => {
});
```

#### Event: 'ping'
<!-- YAML
added: REPLACEME
-->

* `payload` {Buffer} The `PING` frame 8-byte payload

The `'ping'` event is emitted whenever a `PING` frame is received from the
connected peer.

#### Event: 'remoteSettings'
<!-- YAML
added: v8.4.0
Expand Down
10 changes: 10 additions & 0 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,15 @@ function submitRstStream(code) {
}
}

function onPing(payload) {
const session = this[kOwner];
if (session.destroyed)
return;
session[kUpdateTimer]();
debug(`Http2Session ${sessionName(session[kType])}: new ping received`);
session.emit('ping', payload);
}

// Called when the stream is closed either by sending or receiving an
// RST_STREAM frame, or through a natural end-of-stream.
// If the writable and readable sides of the stream are still open at this
Expand Down Expand Up @@ -803,6 +812,7 @@ function setupHandle(socket, type, options) {
handle.error = onSessionInternalError;
handle.onpriority = onPriority;
handle.onsettings = onSettings;
handle.onping = onPing;
handle.onheaders = onSessionHeaders;
handle.onframeerror = onFrameError;
handle.ongoawaydata = onGoawayData;
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ class ModuleWrap;
V(onreadstart_string, "onreadstart") \
V(onreadstop_string, "onreadstop") \
V(onselect_string, "onselect") \
V(onping_string, "onping") \
V(onsettings_string, "onsettings") \
V(onshutdown_string, "onshutdown") \
V(onsignal_string, "onsignal") \
Expand Down
22 changes: 13 additions & 9 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,11 @@ void Http2Session::HandleOriginFrame(const nghttp2_frame* frame) {

// Called by OnFrameReceived when a complete PING frame has been received.
inline void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
Isolate* isolate = env()->isolate();
HandleScope scope(isolate);
Local<Context> context = env()->context();
Context::Scope context_scope(context);
Local<Value> arg;
bool ack = frame->hd.flags & NGHTTP2_FLAG_ACK;
if (ack) {
Http2Ping* ping = PopPing();
Expand All @@ -1479,16 +1484,15 @@ inline void Http2Session::HandlePingFrame(const nghttp2_frame* frame) {
// receive an unsolicited PING ack on a connection. Either the peer
// is buggy or malicious, and we're not going to tolerate such
// nonsense.
Isolate* isolate = env()->isolate();
HandleScope scope(isolate);
Local<Context> context = env()->context();
Context::Scope context_scope(context);

Local<Value> argv[1] = {
Integer::New(isolate, NGHTTP2_ERR_PROTO),
};
MakeCallback(env()->error_string(), arraysize(argv), argv);
arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
MakeCallback(env()->error_string(), 1, &arg);
}
} else {
// Notify the session that a ping occurred
arg = Buffer::Copy(env(),
reinterpret_cast<const char*>(frame->ping.opaque_data),
8).ToLocalChecked();
MakeCallback(env()->onping_string(), 1, &arg);
}
}

Expand Down
48 changes: 48 additions & 0 deletions test/parallel/test-http2-onping.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
'use strict';

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

const {
deepStrictEqual
} = require('assert');
const {
createServer,
connect
} = require('http2');

const check = Buffer.from([ 1, 2, 3, 4, 5, 6, 7, 8 ]);

const server = createServer();
server.on('stream', mustCall((stream) => {
stream.respond();
stream.end('ok');
}));
server.on('session', mustCall((session) => {
session.on('ping', mustCall((payload) => {
deepStrictEqual(check, payload);
}));
session.ping(check, mustCall());
}));
server.listen(0, mustCall(() => {
const client = connect(`http://localhost:${server.address().port}`);

client.on('ping', mustCall((payload) => {
deepStrictEqual(check, payload);
}));
client.on('connect', mustCall(() => {
client.ping(check, mustCall());
}));

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

0 comments on commit 10576d6

Please sign in to comment.