Skip to content

Commit a5bf7de

Browse files
addaleaxruyadorno
authored andcommittedMar 30, 2021
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 Fixes: #37849 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 364c8ac commit a5bf7de

File tree

1 file changed

+46
-8
lines changed

1 file changed

+46
-8
lines changed
 

‎lib/internal/http2/core.js

+46-8
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,16 @@ const {
1919
Promise,
2020
PromisePrototypeCatch,
2121
ReflectApply,
22+
ReflectGet,
2223
ReflectGetPrototypeOf,
24+
ReflectSet,
2325
RegExpPrototypeTest,
2426
SafeArrayIterator,
2527
SafeMap,
2628
SafeSet,
2729
StringPrototypeSlice,
2830
Symbol,
29-
TypedArrayPrototypeSet,
31+
TypedArrayPrototypeGetLength,
3032
Uint32Array,
3133
Uint8Array,
3234
} = primordials;
@@ -959,6 +961,36 @@ const validateSettings = hideStackFrames((settings) => {
959961
}
960962
});
961963

964+
// Wrap a typed array in a proxy, and allow selectively copying the entries
965+
// that have explicitly been set to another typed array.
966+
function trackAssignmentsTypedArray(typedArray) {
967+
const typedArrayLength = TypedArrayPrototypeGetLength(typedArray);
968+
const modifiedEntries = new Uint8Array(typedArrayLength);
969+
970+
function copyAssigned(target) {
971+
for (let i = 0; i < typedArrayLength; i++) {
972+
if (modifiedEntries[i]) {
973+
target[i] = typedArray[i];
974+
}
975+
}
976+
}
977+
978+
return new Proxy(typedArray, {
979+
get(obj, prop, receiver) {
980+
if (prop === 'copyAssigned') {
981+
return copyAssigned;
982+
}
983+
return ReflectGet(obj, prop, receiver);
984+
},
985+
set(obj, prop, value) {
986+
if (`${+prop}` === prop) {
987+
modifiedEntries[prop] = 1;
988+
}
989+
return ReflectSet(obj, prop, value);
990+
}
991+
});
992+
}
993+
962994
// Creates the internal binding.Http2Session handle for an Http2Session
963995
// instance. This occurs only after the socket connection has been
964996
// established. Note: the binding.Http2Session will take over ownership
@@ -989,10 +1021,13 @@ function setupHandle(socket, type, options) {
9891021
handle.consume(socket._handle);
9901022

9911023
this[kHandle] = handle;
992-
if (this[kNativeFields])
993-
TypedArrayPrototypeSet(handle.fields, this[kNativeFields]);
994-
else
995-
this[kNativeFields] = handle.fields;
1024+
if (this[kNativeFields]) {
1025+
// If some options have already been set before the handle existed, copy
1026+
// those (and only those) that have manually been set over.
1027+
this[kNativeFields].copyAssigned(handle.fields);
1028+
}
1029+
1030+
this[kNativeFields] = handle.fields;
9961031

9971032
if (socket.encrypted) {
9981033
this[kAlpnProtocol] = socket.alpnProtocol;
@@ -1044,7 +1079,8 @@ function cleanupSession(session) {
10441079
session[kProxySocket] = undefined;
10451080
session[kSocket] = undefined;
10461081
session[kHandle] = undefined;
1047-
session[kNativeFields] = new Uint8Array(kSessionUint8FieldCount);
1082+
session[kNativeFields] = trackAssignmentsTypedArray(
1083+
new Uint8Array(kSessionUint8FieldCount));
10481084
if (handle)
10491085
handle.ondone = null;
10501086
if (socket) {
@@ -1212,8 +1248,10 @@ class Http2Session extends EventEmitter {
12121248
setupFn();
12131249
}
12141250

1215-
if (!this[kNativeFields])
1216-
this[kNativeFields] = new Uint8Array(kSessionUint8FieldCount);
1251+
if (!this[kNativeFields]) {
1252+
this[kNativeFields] = trackAssignmentsTypedArray(
1253+
new Uint8Array(kSessionUint8FieldCount));
1254+
}
12171255
this.on('newListener', sessionListenerAdded);
12181256
this.on('removeListener', sessionListenerRemoved);
12191257

0 commit comments

Comments
 (0)
Please sign in to comment.