fix(ext/node): wire HTTP/2 PING ack callbacks and emit payload buffer (#33794)

Enables `test-http2-onping` in node_compat suite.

Co-authored-by: Divy Srivastava <me@littledivy.com>

Author: divybot

ext/node/ops/http2/session.rs

@@ -834,12 +834,12 @@ unsafe extern "C" fn on_frame_recv_callback(
       // destroy the session with NghttpError(NGHTTP2_ERR_PROTO).
       if session.pending_pings > 0 {
         session.pending_pings -= 1;
-        handle_ping_frame(session);
+        handle_ping_frame(session, frame, true);
       } else {
         handle_unsolicited_ping_ack(session);
       }
     } else {
-      handle_ping_frame(session);
+      handle_ping_frame(session, frame, false);
     }
   } else if ft == ffi::NGHTTP2_ALTSVC as u32 {
     handle_alt_svc_frame(session, frame);
@@ -997,22 +997,43 @@ fn handle_settings_frame(session: &Session) {
   callback.call(scope, recv.into(), &[]);
 }
 
-fn handle_ping_frame(session: &Session) {
+fn handle_ping_frame(
+  session: &Session,
+  frame: *const ffi::nghttp2_frame,
+  is_ack: bool,
+) {
   let mut isolate =
     // SAFETY: isolate pointer is valid for the session's lifetime
     unsafe { v8::Isolate::from_raw_isolate_ptr(session.isolate) };
   v8::scope!(let scope, &mut isolate);
   let context = v8::Local::new(scope, session.context.clone());
   let scope = &mut v8::ContextScope::new(scope, context);
 
+  // SAFETY: frame is a valid PING frame per nghttp2 callback contract
+  let opaque = unsafe { (*frame).ping.opaque_data };
+  let array_buffer = v8::ArrayBuffer::new(scope, opaque.len());
+  let backing_store = array_buffer.get_backing_store();
+  if let Some(backing_data) = backing_store.data() {
+    // SAFETY: src and dst are valid, non-overlapping, dst has room for 8 bytes
+    unsafe {
+      std::ptr::copy_nonoverlapping(
+        opaque.as_ptr(),
+        backing_data.as_ptr() as *mut u8,
+        opaque.len(),
+      );
+    }
+  }
+  let payload =
+    v8::Uint8Array::new(scope, array_buffer, 0, opaque.len()).unwrap();
+
   let state = session.op_state.borrow();
   let callbacks = state.borrow::<SessionCallbacks>();
   let recv = v8::Local::new(scope, &session.this);
   let callback = v8::Local::new(scope, &callbacks.ping_frame_cb);
   drop(state);
 
-  let arg = v8::null(scope);
-  callback.call(scope, recv.into(), &[arg.into()]);
+  let is_ack_v = v8::Boolean::new(scope, is_ack);
+  callback.call(scope, recv.into(), &[payload.into(), is_ack_v.into()]);
 }
 
 /// Inbound PING ACK with no matching outstanding PING. Mirrors Node's

ext/node/polyfills/http2.ts

@@ -664,14 +664,31 @@ const proxySocketHandler = {
   },
 };
 
-function onPing(payload) {
+function onPing(payload, isAck) {
   const session = this[kOwner];
   if (session.destroyed) {
     return;
   }
   session[kUpdateTimer]();
   debugSessionObj(session, "new ping received");
-  session.emit("ping", payload);
+  // Convert the Uint8Array payload to a Buffer so userland sees a Buffer
+  // instance (matches Node.js behaviour and allows deepStrictEqual against
+  // Buffers passed by callers).
+  const buf = payload && Buffer.from(
+    payload.buffer,
+    payload.byteOffset,
+    payload.byteLength,
+  );
+  if (isAck) {
+    // Inbound PING ACK - resolve the oldest outstanding ping callback in
+    // submission order (RFC 7540 6.7 requires PINGs to be ack'd in order).
+    // The Rust binding has already validated this is a solicited ACK
+    // (unsolicited ACKs are routed through the internal-error callback).
+    const cb = session[kState].pendingPings.shift();
+    if (cb) cb(true, 0.0, buf);
+    return;
+  }
+  session.emit("ping", buf);
 }
 
 // Called when the stream is closed either by sending or receiving an
@@ -3618,12 +3635,23 @@ class Http2Session extends EventEmitter {
     // didn't supply one.
     const buf = payload || Buffer.alloc(8);
 
+    const ret = this[kHandle].ping(buf);
+    if (ret !== 0) {
+      process.nextTick(cb, false, 0.0, payload);
+      return false;
+    }
     // Track the pending ping so it can be cancelled when the session is
     // destroyed before the PING ACK arrives. Matches Node's
-    // ClearOutstandingPings in src/node_http2.cc.
+    // ClearOutstandingPings in src/node_http2.cc. The wrapped pingCallback
+    // (registered via Http2Session#ping) is consumed FIFO when an inbound
+    // PING ACK is delivered to onPing below.
     this[kState].pendingPings.push(cb);
-
-    return this[kHandle].ping(buf, cb);
+    // The native ping op queues the PING frame inside nghttp2 but the
+    // session's actual transport is owned by the JS socket (setupHandle
+    // overrides handle.sendPending), so we must trigger the flush ourselves
+    // for the PING to leave this peer.
+    scheduleSendPending(this);
+    return true;
   }
 
   [kInspect](depth, opts) {

tests/node_compat/config.jsonc

@@ -1776,6 +1776,7 @@
     "parallel/test-http2-multistream-destroy-on-read-tls.js": {},
     "parallel/test-http2-no-more-streams.js": {},
     "parallel/test-http2-no-wanttrailers-listener.js": {},
+    "parallel/test-http2-onping.js": {},
     "parallel/test-http2-options-max-headers-block-length.js": {},
     "parallel/test-http2-options-max-headers-exceeds-nghttp2.js": {},
     "parallel/test-http2-options-server-request.js": {},