From c3bd0fdb739f833df50540c3504fd6adf936a4f6 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 23 Mar 2021 15:44:21 +0100
Subject: [PATCH] http2: fix setting options before handle exists

Currently, when a JS Http2Session object is created, we have
to handle the situation in which the native object corresponding
to it does not yet exist. As part of that, we create a typed array
for storing options that are passed through the `AliasedStruct`
mechanism, and up until now, we copied that typed array over
the native one once the native one was available.

This was not good, because it was overwriting the defaults that
were set during construction of the native typed array with zeroes.

In order to fix this, create a wrapper for the JS-created typed array
that keeps track of which fields were changed, which enables us to
only overwrite fields that were intentionally changed on the JS side.

It is surprising that this behavior was not tested (which is,
guessing from the commit history around these features, my fault).
The subseqeuent commit introduces a test that would fail without
this change.

PR-URL: https://github.com/nodejs/node/pull/37875
Backport-PR-URL: https://github.com/nodejs/node/pull/38673
Fixes: https://github.com/nodejs/node/issues/37849
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
---
 lib/internal/http2/core.js | 52 +++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 7 deletions(-)

diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index cca254be3f8559..5e930ad216a1f5 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -13,7 +13,9 @@ const {
   ObjectPrototypeHasOwnProperty,
   Promise,
   ReflectApply,
+  ReflectGet,
   ReflectGetPrototypeOf,
+  ReflectSet,
   Set,
   Symbol,
   Uint32Array,
@@ -933,6 +935,36 @@ const validateSettings = hideStackFrames((settings) => {
   }
 });
 
+// Wrap a typed array in a proxy, and allow selectively copying the entries
+// that have explicitly been set to another typed array.
+function trackAssignmentsTypedArray(typedArray) {
+  const typedArrayLength = typedArray.length;
+  const modifiedEntries = new Uint8Array(typedArrayLength);
+
+  function copyAssigned(target) {
+    for (let i = 0; i < typedArrayLength; i++) {
+      if (modifiedEntries[i]) {
+        target[i] = typedArray[i];
+      }
+    }
+  }
+
+  return new Proxy(typedArray, {
+    get(obj, prop, receiver) {
+      if (prop === 'copyAssigned') {
+        return copyAssigned;
+      }
+      return ReflectGet(obj, prop, receiver);
+    },
+    set(obj, prop, value) {
+      if (`${+prop}` === prop) {
+        modifiedEntries[prop] = 1;
+      }
+      return ReflectSet(obj, prop, value);
+    }
+  });
+}
+
 // Creates the internal binding.Http2Session handle for an Http2Session
 // instance. This occurs only after the socket connection has been
 // established. Note: the binding.Http2Session will take over ownership
@@ -963,10 +995,13 @@ function setupHandle(socket, type, options) {
   handle.consume(socket._handle);
 
   this[kHandle] = handle;
-  if (this[kNativeFields])
-    handle.fields.set(this[kNativeFields]);
-  else
-    this[kNativeFields] = handle.fields;
+  if (this[kNativeFields]) {
+    // If some options have already been set before the handle existed, copy
+    // those (and only those) that have manually been set over.
+    this[kNativeFields].copyAssigned(handle.fields);
+  }
+
+  this[kNativeFields] = handle.fields;
 
   if (socket.encrypted) {
     this[kAlpnProtocol] = socket.alpnProtocol;
@@ -1018,7 +1053,8 @@ function cleanupSession(session) {
   session[kProxySocket] = undefined;
   session[kSocket] = undefined;
   session[kHandle] = undefined;
-  session[kNativeFields] = new Uint8Array(kSessionUint8FieldCount);
+  session[kNativeFields] = trackAssignmentsTypedArray(
+    new Uint8Array(kSessionUint8FieldCount));
   if (handle)
     handle.ondone = null;
   if (socket) {
@@ -1184,8 +1220,10 @@ class Http2Session extends EventEmitter {
       setupFn();
     }
 
-    if (!this[kNativeFields])
-      this[kNativeFields] = new Uint8Array(kSessionUint8FieldCount);
+    if (!this[kNativeFields]) {
+      this[kNativeFields] = trackAssignmentsTypedArray(
+        new Uint8Array(kSessionUint8FieldCount));
+    }
     this.on('newListener', sessionListenerAdded);
     this.on('removeListener', sessionListenerRemoved);