Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

worker_threads: add support for data: URLs #34584

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,10 @@ if (isMainThread) {
<!-- YAML
added: v10.5.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34584
description: The `filename` parameter can be a WHATWG `URL` object using
`data:` protocol.
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/34394
description: The `trackUnmanagedFds` option was set to `true` by default.
Expand Down Expand Up @@ -654,7 +658,9 @@ changes:
* `filename` {string|URL} The path to the Worker’s main script or module. Must
be either an absolute path or a relative path (i.e. relative to the
current working directory) starting with `./` or `../`, or a WHATWG `URL`
object using `file:` protocol.
object using `file:` or `data:` protocol.
When using a [`data:` URL][], the data is interpreted based on MIME type using
the [ECMAScript module loader][].
If `options.eval` is `true`, this is a string containing JavaScript code
rather than a path.
* `options` {Object}
Expand Down Expand Up @@ -902,6 +908,7 @@ active handle in the event system. If the worker is already `unref()`ed calling
[`AsyncResource`]: async_hooks.html#async_hooks_class_asyncresource
[`Buffer`]: buffer.html
[`Buffer.allocUnsafe()`]: buffer.html#buffer_static_method_buffer_allocunsafe_size
[ECMAScript module loader]: esm.html#esm_data_imports
[`ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST`]: errors.html#errors_err_missing_message_port_in_transfer_list
[`ERR_WORKER_NOT_RUNNING`]: errors.html#ERR_WORKER_NOT_RUNNING
[`EventEmitter`]: events.html
Expand Down Expand Up @@ -953,3 +960,4 @@ active handle in the event system. If the worker is already `unref()`ed calling
[child processes]: child_process.html
[contextified]: vm.html#vm_what_does_it_mean_to_contextify_an_object
[v8.serdes]: v8.html#v8_serialization_api
[`data:` URL]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URIs
3 changes: 3 additions & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1465,6 +1465,9 @@ E('ERR_WORKER_PATH', (filename) =>
(filename.startsWith('file://') ?
' Wrap file:// URLs with `new URL`.' : ''
) +
(filename.startsWith('data:text/javascript') ?
' Wrap data: URLs with `new URL`.' : ''
) +
` Received "${filename}"`,
TypeError);
E('ERR_WORKER_UNSERIALIZABLE_ERROR',
Expand Down
7 changes: 6 additions & 1 deletion lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ port.on('message', (message) => {
debug(`[${threadId}] starts worker script ${filename} ` +
`(eval = ${eval}) at cwd = ${process.cwd()}`);
port.postMessage({ type: UP_AND_RUNNING });
if (doEval) {
if (doEval === 'classic') {
const { evalScript } = require('internal/process/execution');
const name = '[worker eval]';
// This is necessary for CJS module compilation.
Expand All @@ -161,6 +161,11 @@ port.on('message', (message) => {
});
process.argv.splice(1, 0, name);
evalScript(name, filename);
} else if (doEval === 'module') {
const { evalModule } = require('internal/process/execution');
evalModule(filename).catch((e) => {
workerOnGlobalUncaughtException(e, true);
});
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
} else {
// script filename
// runMain here might be monkey-patched by users in --require.
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/process/execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function evalModule(source, print) {
const { log } = require('internal/console/global');
const { loadESM } = require('internal/process/esm_loader');
const { handleMainPromise } = require('internal/modules/run_main');
handleMainPromise(loadESM(async (loader) => {
return handleMainPromise(loadESM(async (loader) => {
const { result } = await loader.eval(source);
if (print) {
log(result);
Expand Down
11 changes: 9 additions & 2 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

const {
ArrayIsArray,
JSONStringify,
MathMax,
ObjectCreate,
ObjectEntries,
Expand Down Expand Up @@ -100,7 +101,7 @@ class Worker extends EventEmitter {
argv = options.argv.map(String);
}

let url;
let url, doEval;
if (options.eval) {
if (typeof filename !== 'string') {
throw new ERR_INVALID_ARG_VALUE(
Expand All @@ -110,7 +111,13 @@ class Worker extends EventEmitter {
);
}
url = null;
doEval = 'classic';
} else if (isURLInstance(filename) && filename.protocol === 'data:') {
url = null;
doEval = 'module';
filename = `import ${JSONStringify(`${filename}`)}`;
} else {
doEval = false;
if (isURLInstance(filename)) {
url = filename;
filename = fileURLToPath(filename);
Expand Down Expand Up @@ -201,7 +208,7 @@ class Worker extends EventEmitter {
argv,
type: messageTypes.LOAD_SCRIPT,
filename,
doEval: !!options.eval,
doEval,
cwdCounter: cwdCounter || workerIo.sharedCwdCounter,
workerData: options.workerData,
publicPort: port2,
Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-worker-data-url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';

const common = require('../common');
const { Worker } = require('worker_threads');
const assert = require('assert');

new Worker(new URL('data:text/javascript,'))
.on('error', common.mustNotCall(() => {}));
new Worker(new URL('data:text/javascript,export{}'))
.on('error', common.mustNotCall(() => {}));

new Worker(new URL('data:text/plain,'))
.on('error', common.mustCall(() => {}));
new Worker(new URL('data:text/javascript,module.exports={}'))
.on('error', common.mustCall(() => {}));

new Worker(new URL('data:text/javascript,await Promise.resolve()'))
.on('error', common.mustNotCall(() => {}));
new Worker(new URL('data:text/javascript,await Promise.reject()'))
.on('error', common.mustCall(() => {}));
new Worker(new URL('data:text/javascript,await new Promise(()=>{})'))
.on(
'exit',
common.mustCall((exitCode) => { assert.strictEqual(exitCode, 13); })
);
6 changes: 4 additions & 2 deletions test/parallel/test-worker-unsupported-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ const { Worker } = require('worker_threads');
() => { new Worker('file:///file_url'); },
/Wrap file:\/\/ URLs with `new URL`/
);
assert.throws(
() => { new Worker('data:text/javascript,'); },
/Wrap data: URLs with `new URL`/
);
assert.throws(
() => { new Worker('relative_no_dot'); },
// eslint-disable-next-line node-core/no-unescaped-regexp-dot
Expand All @@ -47,6 +51,4 @@ const { Worker } = require('worker_threads');
};
assert.throws(() => { new Worker(new URL('https://www.url.com')); },
expectedErr);
assert.throws(() => { new Worker(new URL('data:application/javascript,')); },
expectedErr);
}