http2: add ping event

Add a `Http2Session` event whenever a non-ack `PING` is received. Fixes: nodejs#18514 PR-URL: nodejs#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]>
linvinglor · Sep 23, 2018 · a0c1326 · a0c1326
1 parent c67d3d0
commit a0c1326
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 5 deletions.
diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -222,6 +222,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

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -346,6 +346,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
@@ -821,6 +830,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;

diff --git a/src/env.h b/src/env.h
@@ -229,6 +229,7 @@ struct PackageConfig {
   V(onread_string, "onread")                                                  \
   V(onreadstart_string, "onreadstart")                                        \
   V(onreadstop_string, "onreadstop")                                          \
+  V(onping_string, "onping")                                                  \
   V(onsettings_string, "onsettings")                                          \
   V(onshutdown_string, "onshutdown")                                          \
   V(onsignal_string, "onsignal")                                              \

diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -1457,6 +1457,11 @@ void Http2Session::HandleOriginFrame(const nghttp2_frame* frame) {
 
 // Called by OnFrameReceived when a complete PING frame has been received.
 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();
@@ -1468,13 +1473,15 @@ 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> arg = Integer::New(isolate, NGHTTP2_ERR_PROTO);
+      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);
   }
 }
 

diff --git a/test/parallel/test-http2-onping.js b/test/parallel/test-http2-onping.js
@@ -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();
+  }));
+}));