JSRPC: Support "serializing" functions by turning them into stubs

WORKSPACE

@@ -450,6 +450,7 @@ http_archive(
         "//:patches/v8/0012-Always-enable-continuation-preserved-data-in-the-bui.patch",
         "//:patches/v8/0013-Attach-continuation-context-to-Promise-thenable-task.patch",
         "//:patches/v8/0014-increase-visibility-of-virtual-method.patch",
+        "//:patches/v8/0015-Add-ValueSerializer-SetTreatFunctionsAsHostObjects.patch",
     ],
     integrity = "sha256-jcBk1hBhzrMHRL0EDTgHKBVrJPsP1SLZL6A5/l6arrs=",
     strip_prefix = "v8-12.2.281.18",

patches/v8/0015-Add-ValueSerializer-SetTreatFunctionsAsHostObjects.patch

@@ -0,0 +1,103 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Kenton Varda <[email protected]>
+Date: Sat, 2 Mar 2024 09:00:18 -0600
+Subject: Add ValueSerializer::SetTreatFunctionsAsHostObjects().
+
+Previously, ValueSerializer would always refuse to serialize functions. This commit gives the embedder the option to handle them as host objects.
+
+This is intended for use in an RPC system, where a function can be "serialized" by replacing it with a stub which, when invoked, performs an RPC back to the originating isolate in order to execute the original function there.
+
+diff --git a/include/v8-value-serializer.h b/include/v8-value-serializer.h
+index 596be18adeb3a5a81794aaa44b1d347dec6c0c7d..141f138e08de849e3e02b3b2b346e643b9e40c70 100644
+--- a/include/v8-value-serializer.h
++++ b/include/v8-value-serializer.h
+@@ -195,6 +195,15 @@ class V8_EXPORT ValueSerializer {
+    */
+   void SetTreatArrayBufferViewsAsHostObjects(bool mode);
+
++  /**
++   * Indicate whether to treat Functions as host objects,
++   * i.e. pass them to Delegate::WriteHostObject. This should not be
++   * called when no Delegate was passed.
++   *
++   * The default is not to treat Functions as host objects.
++   */
++  void SetTreatFunctionsAsHostObjects(bool mode);
++
+   /**
+    * Write raw data in various common formats to the buffer.
+    * Note that integer types are written in base-128 varint format, not with a
+diff --git a/src/api/api.cc b/src/api/api.cc
+index 336aaf5512f743d1202d503e388d2ac9ef5a9377..d736a92e87c758a82247af6623d5a706c646c978 100644
+--- a/src/api/api.cc
++++ b/src/api/api.cc
+@@ -3577,6 +3577,10 @@ void ValueSerializer::SetTreatArrayBufferViewsAsHostObjects(bool mode) {
+   private_->serializer.SetTreatArrayBufferViewsAsHostObjects(mode);
+ }
+
++void ValueSerializer::SetTreatFunctionsAsHostObjects(bool mode) {
++  private_->serializer.SetTreatFunctionsAsHostObjects(mode);
++}
++
+ Maybe<bool> ValueSerializer::WriteValue(Local<Context> context,
+                                         Local<Value> value) {
+   auto i_isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
+diff --git a/src/objects/value-serializer.cc b/src/objects/value-serializer.cc
+index f0b1956253fe603ad6da30a41fb923078ef6bd78..23eff3120d5b20ce4ac1ec8d3700e82a49150308 100644
+--- a/src/objects/value-serializer.cc
++++ b/src/objects/value-serializer.cc
+@@ -304,6 +304,10 @@ void ValueSerializer::SetTreatArrayBufferViewsAsHostObjects(bool mode) {
+   treat_array_buffer_views_as_host_objects_ = mode;
+ }
+
++void ValueSerializer::SetTreatFunctionsAsHostObjects(bool mode) {
++  treat_functions_as_host_objects_ = mode;
++}
++
+ void ValueSerializer::WriteTag(SerializationTag tag) {
+   uint8_t raw_tag = static_cast<uint8_t>(tag);
+   WriteRawBytes(&raw_tag, sizeof(raw_tag));
+@@ -572,8 +576,13 @@ Maybe<bool> ValueSerializer::WriteJSReceiver(Handle<JSReceiver> receiver) {
+
+   // Eliminate callable and exotic objects, which should not be serialized.
+   InstanceType instance_type = receiver->map()->instance_type();
+-  if (IsCallable(*receiver) || (IsSpecialReceiverInstanceType(instance_type) &&
+-                                instance_type != JS_SPECIAL_API_OBJECT_TYPE)) {
++  if (IsCallable(*receiver)) {
++    if (treat_functions_as_host_objects_) {
++      return WriteHostObject(Handle<JSObject>::cast(receiver));
++    }
++    return ThrowDataCloneError(MessageTemplate::kDataCloneError, receiver);
++  } else if (IsSpecialReceiverInstanceType(instance_type) &&
++             instance_type != JS_SPECIAL_API_OBJECT_TYPE) {
+     return ThrowDataCloneError(MessageTemplate::kDataCloneError, receiver);
+   }
+
+diff --git a/src/objects/value-serializer.h b/src/objects/value-serializer.h
+index 4747119155007e1fa0075440fefa1dfee036c48a..68be2829f36633a386f54668ce9147267c377272 100644
+--- a/src/objects/value-serializer.h
++++ b/src/objects/value-serializer.h
+@@ -102,6 +102,15 @@ class ValueSerializer {
+    */
+   void SetTreatArrayBufferViewsAsHostObjects(bool mode);
+
++  /*
++   * Indicate whether to treat Functions as host objects,
++   * i.e. pass them to Delegate::WriteHostObject. This should not be
++   * called when no Delegate was passed.
++   *
++   * The default is not to treat Functions as host objects.
++   */
++  void SetTreatFunctionsAsHostObjects(bool mode);
++
+  private:
+   // Managing allocations of the internal buffer.
+   Maybe<bool> ExpandBuffer(size_t required_capacity);
+@@ -181,6 +190,7 @@ class ValueSerializer {
+   size_t buffer_capacity_ = 0;
+   bool has_custom_host_objects_ = false;
+   bool treat_array_buffer_views_as_host_objects_ = false;
++  bool treat_functions_as_host_objects_ = false;
+   bool out_of_memory_ = false;
+   Zone zone_;
+   uint32_t version_;

src/node/internal/internal_assert.ts

@@ -846,10 +846,7 @@ function isValidThenable(maybeThennable: any): boolean {
     return true;
   }
 
-  const isThenable = typeof maybeThennable.then === "function" &&
-    typeof maybeThennable.catch === "function";
-
-  return isThenable && typeof maybeThennable !== "function";
+  return typeof maybeThennable.then === "function";
 }
 
 export { AssertionError };

src/workerd/api/tests/js-rpc-test.js

@@ -105,13 +105,17 @@ export class MyService extends WorkerEntrypoint {
   get functionProperty() { return (a, b) => a - b; }
 
   get objectProperty() {
+    let nullPrototype = {foo: 123};
+    nullPrototype.__proto__ = null;
+
     return {
       func: (a, b) => a * b,
       deeper: {
         deepFunc: (a, b) => a / b,
       },
       counter5: new MyCounter(5),
       nonRpc: new NonRpcClass(),
+      nullPrototype,
       someText: "hello",
     };
   }
@@ -138,6 +142,49 @@ export class MyService extends WorkerEntrypoint {
   async tryUseGlobalRpcPromisePipeline() {
     return await globalRpcPromise.increment(1);
   }
+
+  async getNonRpcClass() {
+    return {obj: new NonRpcClass()};
+  }
+
+  async getNullPrototypeObject() {
+    let obj = {foo: 123};
+    obj.__proto__ = null;
+    return obj;
+  }
+
+  async getFunction() {
+    let func = (a, b) => a ^ b;
+    func.someProperty = 123;
+    return func;
+  }
+
+  getRpcPromise(callback) {
+    return callback();
+  }
+  getNestedRpcPromise(callback) {
+    return {value: callback()};
+  }
+  getRemoteNestedRpcPromise(callback) {
+    // Use a function as a cheap way to return a JsRpcStub that has a remote property `value` which
+    // itself is initialized as a JsRpcPromise.
+    let result = () => {};
+    result.value = callback();
+    return result;
+  }
+  getRpcProperty(callback) {
+    return callback.foo;
+  }
+  getNestedRpcProperty(callback) {
+    return {value: callback.foo};
+  }
+  getRemoteNestedRpcProperty(callback) {
+    // Use a function as a cheap way to return a JsRpcStub that has a remote property `value` which
+    // itself is initialized as a JsRpcProperty.
+    let result = () => {};
+    result.value = callback.foo;
+    return result;
+  }
 }
 
 export class MyActor extends DurableObject {
@@ -237,6 +284,22 @@ export let namedServiceBinding = {
       assert.strictEqual(await counter.increment(7), 15);
     }
 
+    {
+      let func = await env.MyService.objectProperty.func;
+      assert.strictEqual(await func(3, 7), 21);
+    }
+    {
+      let func = await env.MyService.getFunction();
+      assert.strictEqual(await func(3, 6), 5);
+      assert.strictEqual(await func.someProperty, 123);
+    }
+    {
+      // Pipeline the function call.
+      let func = env.MyService.getFunction();
+      assert.strictEqual(await func(3, 6), 5);
+      assert.strictEqual(await func.someProperty, 123);
+    }
+
     // A property that returns a Promise will wait for the Promise.
     assert.strictEqual(await env.MyService.promiseProperty, 123);
 
@@ -259,21 +322,21 @@ export let namedServiceBinding = {
       message: "The RPC receiver does not implement the method \"instanceObject\"."
     });
 
-    await assert.rejects(() => Promise.resolve(env.MyService.instanceObject), {
+    await assert.rejects(() => env.MyService.instanceObject, {
       name: "TypeError",
       message: "The RPC receiver does not implement the method \"instanceObject\"."
     });
 
-    await assert.rejects(() => Promise.resolve(env.MyService.throwingProperty), {
+    await assert.rejects(() => env.MyService.throwingProperty, {
       name: "Error",
       message: "PROPERTY THREW"
     });
-    await assert.rejects(() => Promise.resolve(env.MyService.throwingMethod()), {
+    await assert.rejects(() => env.MyService.throwingMethod(), {
       name: "Error",
       message: "METHOD THREW"
     });
 
-    await assert.rejects(() => Promise.resolve(env.MyService.rejectingPromiseProperty), {
+    await assert.rejects(() => env.MyService.rejectingPromiseProperty, {
       name: "Error",
       message: "REJECTED"
     });
@@ -341,6 +404,10 @@ export let namedServiceBinding = {
       name: "TypeError",
       message: "The RPC receiver does not implement the method \"nonRpc\"."
     });
+    await assert.rejects(() => env.MyService.objectProperty.nullPrototype.foo, {
+      name: "TypeError",
+      message: "The RPC receiver does not implement the method \"nullPrototype\"."
+    });
 
     // Extra-paranoid check that we can't access methods on env or ctx.
     await assert.rejects(() => env.MyService.objectProperty.env.MyService.noArgsMethod(), {
@@ -351,6 +418,18 @@ export let namedServiceBinding = {
       name: "TypeError",
       message: "The RPC receiver does not implement the method \"ctx\"."
     });
+
+    // Can't serialize instances of classes that aren't derived from RpcTarget.
+    await assert.rejects(() => env.MyService.getNonRpcClass(), {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "NonRpcClass". This type does not support ' +
+               'serialization.'
+    });
+    await assert.rejects(() => env.MyService.getNullPrototypeObject(), {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "Object". This type does not support ' +
+               'serialization.'
+    });
   },
 }
 
@@ -484,6 +563,80 @@ export let crossContextSharingDoesntWork = {
   },
 }
 
+export let serializeRpcPromiseOrProprety = {
+  async test(controller, env, ctx) {
+    // What happens if we actually try to serialize a JsRpcPromise or JsRpcProperty? Let's make
+    // sure these aren't, for instance, treated as functions because they are callable.
+
+    let func = () => {return {x: 123};};
+    func.foo = {x: 456};
+
+    // If we directly return returning a JsRpcPromise, the system automatically awaits it on the
+    // server side because it's a thenable.
+    assert.deepEqual(await env.MyService.getRpcPromise(func), {x: 123})
+
+    // Pipelining also works.
+    assert.strictEqual(await env.MyService.getRpcPromise(func).x, 123)
+
+    // If a JsRpcPromise appears somewhere in the serialization tree, it'll just fail serialization.
+    // NOTE: We could choose to make this work later.
+    await assert.rejects(() => env.MyService.getNestedRpcPromise(func), {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "JsRpcPromise". This type does not support ' +
+               'serialization.'
+    });
+    await assert.rejects(() => env.MyService.getNestedRpcPromise(func).value, {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "JsRpcPromise". This type does not support ' +
+               'serialization.'
+    });
+    await assert.rejects(() => env.MyService.getNestedRpcPromise(func).value.x, {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "JsRpcPromise". This type does not support ' +
+               'serialization.'
+    });
+
+    // Things get a little weird when we return a stub which itself has properties that reflect
+    // our RPC promise. If we await fetch the JsRpcPromise itself, this works, again because
+    // somewhere along the line V8 says "oh look a thenable" and awaits it, before it can be
+    // subject to serialization. That's fine.
+    assert.deepEqual(await env.MyService.getRemoteNestedRpcPromise(func).value, {x: 123});
+    await assert.rejects(() => env.MyService.getRemoteNestedRpcPromise(func).value.x, {
+      name: "TypeError",
+      message: 'The RPC receiver does not implement the method "value".'
+    });
+
+    // The story is similar for a JsRpcProperty -- though the implementation details differ.
+    assert.deepEqual(await env.MyService.getRpcProperty(func), {x: 456})
+    assert.strictEqual(await env.MyService.getRpcProperty(func).x, 456)
+    await assert.rejects(() => env.MyService.getNestedRpcProperty(func), {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "JsRpcProperty". This type does not support ' +
+               'serialization.'
+    });
+    await assert.rejects(() => env.MyService.getNestedRpcProperty(func).value, {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "JsRpcProperty". This type does not support ' +
+               'serialization.'
+    });
+    await assert.rejects(() => env.MyService.getNestedRpcProperty(func).value.x, {
+      name: "DataCloneError",
+      message: 'Could not serialize object of type "JsRpcProperty". This type does not support ' +
+               'serialization.'
+    });
+
+    assert.deepEqual(await env.MyService.getRemoteNestedRpcProperty(func).value, {x: 456});
+    await assert.rejects(() => env.MyService.getRemoteNestedRpcProperty(func).value.x, {
+      name: "TypeError",
+      message: 'The RPC receiver does not implement the method "value".'
+    });
+    await assert.rejects(() => env.MyService.getRemoteNestedRpcProperty(func).value(), {
+      name: "TypeError",
+      message: '"value" is not a function.'
+    });
+  },
+}
+
 // Test that exceptions thrown from async native functions have a proper stack trace. (This is
 // not specific to RPC but RPC is a convenient place to test it since we can easily define the
 // callee to throw an exception.)