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: #37875
Backport-PR-URL: #38673
Fixes: #37849
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>

Author: addaleax

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);