Harden Isolate ops API

[bartlomieju]

New Isolate::register_op API (#3002) still needs some work, namely:

• OpRegistry::call_op will panic if you try to call op that doesn't exist - it should be handled gracefully by throwing error in JS (eg. Deno.core.send called with unknown op id: 100)
  

  deno/core/ops.rs

  Lines 66 to 81 in 97d8498

  	pub fn call(
  	&self,
  	op_id: OpId,
  	control: &[u8],
  	zero_copy_buf: Option<PinnedBuf>,
  	) -> CoreOp {
  	// Op with id 0 has special meaning - it's a special op that is always
  	// provided to retrieve op id map. The map consists of name to `OpId`
  	// mappings.
  	if op_id == 0 {
  	return Op::Sync(self.json_map());
  	}
  	let d = &*self.dispatchers.get(op_id as usize).expect("Op not found!");
  	d(control, zero_copy_buf)
  	}

• No handling of malformed control buffer in either of dispatch mechanisms.
  

  deno/cli/ops/dispatch_minimal.rs

  Lines 74 to 81 in 97d8498

  	pub fn minimal_op(
  	d: Dispatcher,
  	) -> impl Fn(&[u8], Option<PinnedBuf>) -> CoreOp {
  	move |control: &[u8], zero_copy: Option<PinnedBuf>| {
  	let mut record = parse_min_record(control).unwrap();
  	let is_sync = record.promise_id == 0;
  	let rid = record.arg;
  	let min_op = d(rid, zero_copy);

  
  

  deno/cli/ops/dispatch_json.rs

  Lines 46 to 53 in 97d8498

  	pub fn json_op<D>(d: D) -> impl Fn(&[u8], Option<PinnedBuf>) -> CoreOp
  	where
  	D: Fn(Value, Option<PinnedBuf>) -> Result<JsonOp, ErrBox>,
  	{
  	move |control: &[u8], zero_copy: Option<PinnedBuf>| {
  	let async_args: AsyncArgs = serde_json::from_slice(control).unwrap();
  	let promise_id = async_args.promise_id;
  	let is_sync = promise_id.is_none();

  
  They would panic if you send bad data, eg.

Deno.core.send(1, new Uint8Array([1])
thread 'tokio-runtime-worker-3' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:378:21
stack backtrace:
   0: std::panicking::default_hook::{{closure}}
   1: std::panicking::default_hook
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::continue_panic_fmt
   4: rust_begin_unwind
   5: core::panicking::panic_fmt
   6: core::panicking::panic
   7: core::option::Option<T>::unwrap
   8: deno_cli::ops::dispatch_minimal::minimal_op::{{closure}}
   9: deno_cli::state::ThreadSafeState::cli_op::{{closure}}
  10: <alloc::boxed::Box<F> as core::ops::function::Fn<A>>::call
  11: deno::ops::OpRegistry::call
  12: deno::isolate::Isolate::pre_dispatch
  13: _ZN4deno4SendERKN2v820FunctionCallbackInfoINS0_5ValueEEE
  14: _ZN2v88internal25FunctionCallbackArguments4CallENS0_15CallHandlerInfoE
  15: _ZN2v88internal12_GLOBAL__N_119HandleApiCallHelperILb0EEENS0_11MaybeHandleINS0_6ObjectEEEPNS0_7IsolateENS0_6HandleINS0_10HeapObjectEEESA_NS8_INS0_20FunctionTemplateInfoEEENS8_IS4_EENS0_16BuiltinArgumentsE
  16: _ZN2v88internalL26Builtin_Impl_HandleApiCallENS0_16BuiltinArgumentsEPNS0_7IsolateE
  17: _ZN2v88internal21Builtin_HandleApiCallEiPmPNS0_7IsolateE
fatal runtime error: failed to initiate panic, error 5
Abort trap: 6

Again this should throw an error in JS.

I say this is a prerequisite for native plugins.

Related issues:

• Deno.core.send(new Uint8Array([1]), new Uint8Array([1])) crashes #2459
• Dispatch and native plugins #2987