Skip to content

Provide option to delete Deno namespace in worker #2717

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

Merged
merged 4 commits into from
Aug 5, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,10 @@ fn create_worker_and_state(
s.status(status, msg).expect("shell problem");
}
});
// TODO(kevinkassimo): maybe make include_deno_namespace also configurable?
let state =
ThreadSafeState::new(flags, argv, ops::op_selector_std, progress).unwrap();
ThreadSafeState::new(flags, argv, ops::op_selector_std, progress, true)
.unwrap();
let worker = Worker::new(
"main".to_string(),
startup_data::deno_isolate_init(),
Expand Down
1 change: 1 addition & 0 deletions cli/msg.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ table FormatErrorRes {
// Create worker as host
table CreateWorker {
specifier: string;
include_deno_namespace: bool;
}

table CreateWorkerRes {
Expand Down
8 changes: 7 additions & 1 deletion cli/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2069,6 +2069,10 @@ fn op_create_worker(
let cmd_id = base.cmd_id();
let inner = base.inner_as_create_worker().unwrap();
let specifier = inner.specifier().unwrap();
// Only include deno namespace if requested AND current worker
// has included namespace (to avoid escalation).
let include_deno_namespace =
inner.include_deno_namespace() && state.include_deno_namespace;

let parent_state = state.clone();

Expand All @@ -2077,13 +2081,15 @@ fn op_create_worker(
parent_state.argv.clone(),
op_selector_std,
parent_state.progress.clone(),
include_deno_namespace,
)?;
let rid = child_state.resource.rid;
let name = format!("USER-WORKER-{}", specifier);
let deno_main_call = format!("denoMain({})", include_deno_namespace);

let mut worker =
Worker::new(name, startup_data::deno_isolate_init(), child_state);
worker.execute("denoMain()").unwrap();
worker.execute(&deno_main_call).unwrap();
worker.execute("workerMain()").unwrap();

let module_specifier = ModuleSpecifier::resolve_url_or_path(specifier)?;
Expand Down
5 changes: 5 additions & 0 deletions cli/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub struct State {
pub js_compiler: JsCompiler,
pub json_compiler: JsonCompiler,
pub ts_compiler: TsCompiler,

pub include_deno_namespace: bool,
}

impl Clone for ThreadSafeState {
Expand Down Expand Up @@ -151,6 +153,7 @@ impl ThreadSafeState {
argv_rest: Vec<String>,
dispatch_selector: ops::OpSelector,
progress: Progress,
include_deno_namespace: bool,
) -> Result<Self, ErrBox> {
let custom_root = env::var("DENO_DIR").map(String::into).ok();

Expand Down Expand Up @@ -223,6 +226,7 @@ impl ThreadSafeState {
ts_compiler,
js_compiler: JsCompiler {},
json_compiler: JsonCompiler {},
include_deno_namespace,
};

Ok(ThreadSafeState(Arc::new(state)))
Expand Down Expand Up @@ -302,6 +306,7 @@ impl ThreadSafeState {
argv,
ops::op_selector_std,
Progress::new(),
true,
)
.unwrap()
}
Expand Down
4 changes: 3 additions & 1 deletion cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ mod tests {
argv,
op_selector_std,
Progress::new(),
true,
)
.unwrap();
let state_ = state.clone();
Expand Down Expand Up @@ -155,6 +156,7 @@ mod tests {
argv,
op_selector_std,
Progress::new(),
true,
)
.unwrap();
let state_ = state.clone();
Expand Down Expand Up @@ -182,7 +184,7 @@ mod tests {
let mut flags = flags::DenoFlags::default();
flags.reload = true;
let state =
ThreadSafeState::new(flags, argv, op_selector_std, Progress::new())
ThreadSafeState::new(flags, argv, op_selector_std, Progress::new(), true)
.unwrap();
let state_ = state.clone();
tokio_util::run(lazy(move || {
Expand Down
6 changes: 5 additions & 1 deletion core/shared_queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@ SharedQueue Binary Layout
const INDEX_RECORDS = 3 + MAX_RECORDS;
const HEAD_INIT = 4 * INDEX_RECORDS;

// Available on start due to bindings.
const Deno = window[GLOBAL_NAMESPACE];
const core = Deno[CORE_NAMESPACE];
// Warning: DO NOT use window.Deno after this point.
// It is possible that the Deno namespace has been deleted.
// Use the above local Deno and core variable instead.

let sharedBytes;
let shared32;
Expand Down Expand Up @@ -165,7 +169,7 @@ SharedQueue Binary Layout
const success = push(control);
// If successful, don't use first argument of core.send.
const arg0 = success ? null : control;
return window.Deno.core.send(arg0, zeroCopy);
return Deno.core.send(arg0, zeroCopy);
}

const denoCore = {
Expand Down
2 changes: 1 addition & 1 deletion js/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const console = new Console(core.print);
window.console = console;
window.workerMain = workerMain;
export default function denoMain(): void {
os.start("TS");
os.start(true, "TS");
}

const ASSETS = "$asset$";
Expand Down
2 changes: 2 additions & 0 deletions js/core.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.
import { window } from "./window";

// This allows us to access core in API even if we
// dispose window.Deno
export const core = window.Deno.core as DenoCore;
3 changes: 1 addition & 2 deletions js/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import * as request from "./request";
// These imports are not exposed and therefore are fine to just import the
// symbols required.
import { core } from "./core";
import { immutableDefine } from "./util";

// During the build process, augmentations to the variable `window` in this
// file are tracked and created as part of default library that is built into
Expand Down Expand Up @@ -71,7 +70,7 @@ window.window = window;
// This is the Deno namespace, it is handled differently from other window
// properties when building the runtime type library, as the whole module
// is flattened into a single namespace.
immutableDefine(window, "Deno", deno);
window.Deno = deno;
Object.freeze(window.Deno);

// Globally available functions and object instances.
Expand Down
7 changes: 5 additions & 2 deletions js/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ import { setLocation } from "./location";
// builtin modules
import * as deno from "./deno";

export default function denoMain(name?: string): void {
const startResMsg = os.start(name);
export default function denoMain(
preserveDenoNamespace: boolean = true,
name?: string
): void {
const startResMsg = os.start(preserveDenoNamespace, name);

setVersions(startResMsg.denoVersion()!, startResMsg.v8Version()!);

Expand Down
20 changes: 16 additions & 4 deletions js/os.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,10 @@ function sendStart(): msg.StartRes {
// This function bootstraps an environment within Deno, it is shared both by
// the runtime and the compiler environments.
// @internal
export function start(source?: string): msg.StartRes {
export function start(
preserveDenoNamespace = true,
source?: string
): msg.StartRes {
core.setAsyncHandler(handleAsyncMsgFromRust);

// First we send an empty `Start` message to let the privileged side know we
Expand All @@ -124,9 +127,18 @@ export function start(source?: string): msg.StartRes {

setGlobals(startResMsg.pid(), startResMsg.noColor(), startResMsg.execPath()!);

// Deno.core could ONLY be safely frozen here (not in globals.ts)
// since shared_queue.js will modify core properties.
Object.freeze(window.Deno.core);
if (preserveDenoNamespace) {
util.immutableDefine(window, "Deno", window.Deno);
// Deno.core could ONLY be safely frozen here (not in globals.ts)
// since shared_queue.js will modify core properties.
Object.freeze(window.Deno.core);
// core.sharedQueue is an object so we should also freeze it.
Object.freeze(window.Deno.core.sharedQueue);
} else {
// Remove window.Deno
delete window.Deno;
assert(window.Deno === undefined);
}

return startResMsg;
}
Expand Down
30 changes: 26 additions & 4 deletions js/workers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,17 @@ export function decodeMessage(dataIntArray: Uint8Array): any {
return JSON.parse(dataJson);
}

function createWorker(specifier: string): number {
function createWorker(
specifier: string,
includeDenoNamespace: boolean
): number {
const builder = flatbuffers.createBuilder();
const specifier_ = builder.createString(specifier);
const inner = msg.CreateWorker.createCreateWorker(builder, specifier_);
const inner = msg.CreateWorker.createCreateWorker(
builder,
specifier_,
includeDenoNamespace
);
const baseRes = sendSync(builder, msg.Any.CreateWorker, inner);
assert(baseRes != null);
assert(
Expand Down Expand Up @@ -149,6 +156,17 @@ export interface Worker {
closed: Promise<void>;
}

// TODO(kevinkassimo): Maybe implement reasonable web worker options?
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface WorkerOptions {}

export interface DenoWorkerOptions extends WorkerOptions {
deno?: {
// TODO(kevinkassimo): maybe just name this option "sandbox"?
noDenoNamespace?: boolean;
};
}

export class WorkerImpl implements Worker {
private readonly rid: number;
private isClosing: boolean = false;
Expand All @@ -157,8 +175,12 @@ export class WorkerImpl implements Worker {
public onmessage?: (data: any) => void;
public onmessageerror?: () => void;

constructor(specifier: string) {
this.rid = createWorker(specifier);
constructor(specifier: string, options?: DenoWorkerOptions) {
let includeDenoNamespace = true;
if (options && options.deno && options.deno.noDenoNamespace) {
includeDenoNamespace = false;
}
this.rid = createWorker(specifier, includeDenoNamespace);
this.run();
this.isClosedPromise = hostGetWorkerClosed(this.rid);
this.isClosedPromise.then(
Expand Down