From 8bf004b96d9f12a68d1368c3ac3dea1ac0321ed2 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Fri, 21 Sep 2018 14:14:04 -0700
Subject: [PATCH] http2: add ping event

Add a `Http2Session` event whenever a non-ack `PING` is received.

Fixes: https://github.com/nodejs/node/issues/18514

PR-URL: https://github.com/nodejs/node/pull/23009
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
---
 doc/api/http2.md                   | 10 +++++++
 lib/internal/http2/core.js         | 10 +++++++
 src/env.h                          |  1 +
 src/node_http2.cc                  | 17 +++++++----
 test/parallel/test-http2-onping.js | 48 ++++++++++++++++++++++++++++++
 5 files changed, 81 insertions(+), 5 deletions(-)
 create mode 100644 test/parallel/test-http2-onping.js

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 205c0dc85b8102..698f1bf2d87e58 100644
--- 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
index 6805392488bf90..34558ce459e6d6 100644
--- 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
index b1a3c2b49a08c7..252c828abc1c2b 100644
--- 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
index fa545869047b09..36733fcf6b4d50 100644
--- 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
new file mode 100644
index 00000000000000..134a94ddb8f582
--- /dev/null
+++ 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();
+  }));
+}));