Skip to content

Commit

Permalink
lib: decorate undici classes as platform interfaces
Browse files Browse the repository at this point in the history
Node recognizes platform/host objects by counting internal slots.
Undici, as a downstream module, does not have access to internal slots,
and hence its instances are recognized as plain objects. This caused
issues on the `structureClone` algorithm, which has few restrictions on
non-platform objects.

This PR tries to fix it by decorating Undici classes with the internal
slots so that underlying `Serialize()` can recognize its instances as
host objects.

On another note, this PR consolidates the lazy loading of Undici, so
that the proxied Undici classes are referential equal.
  • Loading branch information
jazelly committed Oct 1, 2024
1 parent c08bb75 commit 448ee28
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 14 deletions.
10 changes: 1 addition & 9 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const {
} = primordials;

const { validateInteger } = require('internal/validators');
const { lazyUndici } = require('internal/util');
const httpAgent = require('_http_agent');
const { ClientRequest } = require('_http_client');
const { methods, parsers } = require('_http_common');
Expand All @@ -42,7 +43,6 @@ const {
ServerResponse,
} = require('_http_server');
let maxHeaderSize;
let undici;

/**
* Returns a new instance of `http.Server`.
Expand Down Expand Up @@ -115,14 +115,6 @@ function get(url, options, cb) {
return req;
}

/**
* Lazy loads WebSocket, CloseEvent and MessageEvent classes from undici
* @returns {object} An object containing WebSocket, CloseEvent, and MessageEvent classes.
*/
function lazyUndici() {
return undici ??= require('internal/deps/undici/undici');
}

module.exports = {
_connectionListener,
METHODS: methods.toSorted(),
Expand Down
38 changes: 37 additions & 1 deletion lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const {
ObjectSetPrototypeOf,
ObjectValues,
Promise,
Proxy,
ReflectApply,
ReflectConstruct,
RegExpPrototypeExec,
Expand Down Expand Up @@ -60,6 +61,7 @@ const {
privateSymbols: {
arrow_message_private_symbol,
decorated_private_symbol,
transfer_mode_private_symbol,
},
sleep: _sleep,
} = internalBinding('util');
Expand Down Expand Up @@ -614,6 +616,31 @@ function exposeGetterAndSetter(target, name, getter, setter = undefined) {
});
}

/**
* Lazy load Undici module by decorating every class with serializable
* private symbol so its instances can be recognized as platform objects
*/
function lazyUndici() {
return getLazy(() => {
const undici = require('internal/deps/undici/undici');
for (const mod of [
'WebSocket', 'EventSource', 'FormData', 'Headers',
'Request', 'Response', 'MessageEvent', 'CloseEvent']) {

undici[mod] = new Proxy(undici[mod], {
__proto__: null,
construct(target, args, newTarget) {
const obj = ReflectConstruct(target, args, newTarget);
// 0 means not cloneable, nor transferable
obj[transfer_mode_private_symbol] = 0;
return obj;
},
});
}
return undici;
})();
}

function defineLazyProperties(target, id, keys, enumerable = true) {
const descriptors = { __proto__: null };
let mod;
Expand All @@ -632,7 +659,15 @@ function defineLazyProperties(target, id, keys, enumerable = true) {
value: `set ${key}`,
});
function get() {
mod ??= require(id);
// Undici is special as it comes from deps and we need to load it with decoration
// TODO(jazelly): not hardcode this. Ideally, every deps module that is
// platform specific needs to be decorated
if (id === 'internal/deps/undici/undici') {
mod = lazyUndici();
} else {
mod ??= require(id);
}

if (lazyLoadedValue === undefined) {
lazyLoadedValue = mod[key];
set(lazyLoadedValue);
Expand Down Expand Up @@ -916,6 +951,7 @@ module.exports = {
join,
lazyDOMException,
lazyDOMExceptionClass,
lazyUndici,
normalizeEncoding,
once,
promisify,
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/wasm_web_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ const {
ERR_WEBASSEMBLY_RESPONSE,
} = require('internal/errors').codes;

let undici;
function lazyUndici() {
return undici ??= require('internal/deps/undici/undici');
}
const { lazyUndici } = require('internal/util');


// This is essentially an implementation of a v8::WasmStreamingCallback, except
// that it is implemented in JavaScript because the fetch() implementation is
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-structuredClone-global.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

// Flags: --experimental-eventsource --no-experimental-websocket --experimental-websocket

require('../common');
const assert = require('assert');

Expand Down Expand Up @@ -30,6 +32,22 @@ for (const StreamClass of [ReadableStream, WritableStream, TransformStream]) {
assert.ok(extendedTransfer instanceof StreamClass);
}

// Platform object that is not serializable should throw
[
{ platformClass: Response, brand: 'Response' },
{ platformClass: Request, value: 'http://localhost', brand: 'Request' },
{ platformClass: FormData, brand: 'FormData' },
{ platformClass: MessageEvent, value: 'message', brand: 'MessageEvent' },
{ platformClass: CloseEvent, value: 'dummy type', brand: 'CloseEvent' },
{ platformClass: WebSocket, value: 'http://localhost', brand: 'WebSocket' },
{ platformClass: EventSource, value: 'http://localhost', brand: 'EventSource' },
].forEach((platformEntity) => {
assert.throws(() => structuredClone(new platformEntity.platformClass(platformEntity.value)),
new DOMException('Cannot clone object of unsupported type.', 'DataCloneError'),
`Cloning ${platformEntity.brand} should throw DOMException`);

});

for (const Transferrable of [File, Blob]) {
const a2 = Transferrable === File ? '' : {};
const original = new Transferrable([], a2);
Expand Down

0 comments on commit 448ee28

Please sign in to comment.