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

Simplify "named intercept" #1585

Merged
merged 9 commits into from
Jan 30, 2024
27 changes: 14 additions & 13 deletions src/workerd/api/worker-rpc.c++
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,14 @@ kj::Promise<capnp::Response<rpc::JsRpcTarget::CallResults>> WorkerRpc::sendWorke
// If we have arguments, serialize them.
// Note that we may fail to serialize some element, in which case this will throw back to JS.
if (argv.size() > 0) {
// TODO(perf): It would be nice if we could serialize directly into the capnp message to avoid
// a redundant copy of the bytes here. Maybe we could even cancel serialization early if it
// goes over the size limit.
auto ser = serializeV8(js, js.arr(argv.asPtr()));
JSG_ASSERT(ser.size() <= MAX_JS_RPC_MESSAGE_SIZE, Error,
"Serialized RPC requests are limited to 1MiB, but the size of this request was: ",
ser.size(), " bytes.");
builder.initSerializedArgs().setV8Serialized(kj::mv(ser));
builder.initArgs().setV8Serialized(ser);
}

auto callResult = builder.send();
Expand All @@ -69,21 +72,19 @@ kj::Promise<capnp::Response<rpc::JsRpcTarget::CallResults>> WorkerRpc::sendWorke
}));
}

kj::Maybe<jsg::JsValue> WorkerRpc::getNamed(jsg::Lock& js, kj::StringPtr name) {
kj::Maybe<WorkerRpc::RpcFunction> WorkerRpc::getRpcMethod(jsg::Lock& js, kj::StringPtr name) {
// Named intercept is enabled, this means we won't default to legacy behavior.
// The return value of the function is a promise that resolves once the remote returns the result
// of the RPC call.
return jsg::JsValue(js.wrapPromiseReturningFunction(js.v8Context(),
[this, methodName=kj::str(name)]
return RpcFunction([this, methodName=kj::str(name)]
(jsg::Lock& js, const v8::FunctionCallbackInfo<v8::Value>& args) -> jsg::Promise<jsg::Value> {
auto& ioContext = IoContext::current();
// Wait for the RPC to resolve and then process the result.
return ioContext.awaitIo(js, sendWorkerRpc(js, methodName, args),
[](jsg::Lock& js, auto result) -> jsg::Value {
return jsg::Value(js.v8Isolate, deserializeV8(js, result.getResult().getV8Serialized()));
});
}
));
auto& ioContext = IoContext::current();
// Wait for the RPC to resolve and then process the result.
return ioContext.awaitIo(js, sendWorkerRpc(js, methodName, args),
[](jsg::Lock& js, auto result) -> jsg::Value {
return jsg::Value(js.v8Isolate, deserializeV8(js, result.getResult().getV8Serialized()));
});
});
}

// The capability that lets us call remote methods over RPC.
Expand All @@ -99,7 +100,7 @@ public:
// Handles the delivery of JS RPC method calls.
kj::Promise<void> call(CallContext callContext) override {
auto methodName = kj::heapString(callContext.getParams().getMethodName());
auto serializedArgs = callContext.getParams().getSerializedArgs().getV8Serialized().asBytes();
auto serializedArgs = callContext.getParams().getArgs().getV8Serialized().asBytes();

// We want to fulfill the callPromise so customEvent can continue executing
// regardless of the outcome of `call()`.
Expand Down
13 changes: 8 additions & 5 deletions src/workerd/api/worker-rpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ constexpr size_t MAX_JS_RPC_MESSAGE_SIZE = 1u << 20;

// A WorkerRpc object forwards JS method calls to the remote Worker/Durable Object over RPC.
// Since methods are not known until runtime, WorkerRpc doesn't define any JS methods.
// Instead, we use JSG_NAMED_INTERCEPT to intercept property accesses of names that are not known at
// compile time.
// Instead, we use JSG_WILDCARD_PROPERTY to intercept property accesses of names that are not known
// at compile time.
//
// WorkerRpc only supports method calls. You cannot, for instance, access a property of a
// Durable Object over RPC.
class WorkerRpc : public Fetcher, public jsg::NamedIntercept {
class WorkerRpc : public Fetcher {
public:
WorkerRpc(
IoOwn<OutgoingFactory> outgoingFactory,
Expand All @@ -50,7 +50,10 @@ class WorkerRpc : public Fetcher, public jsg::NamedIntercept {
kj::StringPtr name,
const v8::FunctionCallbackInfo<v8::Value>& args);

kj::Maybe<jsg::JsValue> getNamed(jsg::Lock& js, kj::StringPtr name) override;
using RpcFunction = jsg::Function<jsg::Promise<jsg::Value>(
const v8::FunctionCallbackInfo<v8::Value>& info)>;

kj::Maybe<RpcFunction> getRpcMethod(jsg::Lock& js, kj::StringPtr name);

// WARNING: Adding a new JSG_METHOD to a class that extends WorkerRpc can conflict with RPC method
// names defined on your remote target. For example, if you add a new method `bar()` to the
Expand All @@ -66,7 +69,7 @@ class WorkerRpc : public Fetcher, public jsg::NamedIntercept {
// change log.
JSG_RESOURCE_TYPE(WorkerRpc, CompatibilityFlags::Reader flags) {
if (flags.getWorkerdExperimental()) {
JSG_NAMED_INTERCEPT();
JSG_WILDCARD_PROPERTY(getRpcMethod);
}
JSG_INHERIT(Fetcher);
}
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/io/worker-interface.capnp
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,11 @@ struct JsValue {
}

interface JsRpcTarget {
call @0 (methodName :Text, serializedArgs :JsValue) -> (result :JsValue);
call @0 (methodName :Text, args :JsValue) -> (result :JsValue);
# Runs a Worker/DO's RPC method.
#
# `methodName` is the name of the method to run, and
# `serializedArgs` is an array of arguments that have been serialized by the v8 serializer.
# `methodName` is the name of the method to run, and `args` is a JsValue that is always a
# JavaScript Array, containing the arguments to the call.
}

interface EventDispatcher @0xf20697475ec1752d {
Expand Down
19 changes: 5 additions & 14 deletions src/workerd/jsg/jsg-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -367,29 +367,24 @@ KJ_TEST("Test JSG_CALLABLE") {

// ========================================================================================
struct InterceptContext: public ContextGlobalObject {
struct ProxyImpl: public jsg::Object,
public jsg::NamedIntercept {
struct ProxyImpl: public jsg::Object {
static jsg::Ref<ProxyImpl> constructor() { return jsg::alloc<ProxyImpl>(); }

int getBar() { return 123; }

// NamedIntercept implementation
kj::Maybe<jsg::JsValue> getNamed(jsg::Lock& js, kj::StringPtr name) override {
// JSG_WILDCARD_PROPERTY implementation
kj::Maybe<kj::StringPtr> testGetNamed(jsg::Lock& js, kj::StringPtr name) {
if (name == "foo") {
return kj::Maybe(js.str("bar"_kj));
return "bar"_kj;
} else if (name == "abc") {
JSG_FAIL_REQUIRE(TypeError, "boom");
}
return kj::none;
}

kj::Array<kj::String> listNamed(Lock& js) override {
return kj::arr(kj::str("foo"));
}

JSG_RESOURCE_TYPE(ProxyImpl) {
JSG_READONLY_PROTOTYPE_PROPERTY(bar, getBar);
JSG_NAMED_INTERCEPT();
JSG_WILDCARD_PROPERTY(testGetNamed);
}
};

Expand All @@ -401,10 +396,6 @@ JSG_DECLARE_ISOLATE_TYPE(InterceptIsolate, InterceptContext, InterceptContext::P

KJ_TEST("Named interceptor") {
Evaluator<InterceptContext, InterceptIsolate> e(v8System);
// Calling Object.keys(p) here just to verify that it does not throw.
// Also, the test tries modifying the known intercepted property foo but verifies
// that the value is readonly/unchanged.
e.expectEval("p = new ProxyImpl; Object.keys(p); p.foo = 123; p.foo", "string", "bar");
e.expectEval("p = new ProxyImpl; p.bar", "number", "123");
e.expectEval("p = new ProxyImpl; Reflect.has(p, 'foo')", "boolean", "true");
e.expectEval("p = new ProxyImpl; Reflect.has(p, 'bar')", "boolean", "true");
Expand Down
21 changes: 17 additions & 4 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -475,11 +475,24 @@ using HasGetTemplateOverload = decltype(
wrapper.initReflection(this, __VA_ARGS__); \
}

// Configures the resource type to implement named property interception.
// @see the definition of jsg::NamedIntercept in resource.h for more information.
#define JSG_NAMED_INTERCEPT() \
// Declares a wildcart property getter. If a property is requested that isn't already present on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/wildcart/wildcard/

// the object or its prototypes, the wildcard property getter will be given a chance to return the
// property.
//
// Example:
//
// struct MyType {
// // Get the value of the named dynamic property. Returns none if the property doesn't exist.
// // `SomeType` can be any type that JSG is able to convert to JavaScript.
// kj::Maybe<SomeType> getWildcard(jsg::Lock& js, kj::StringPtr name);
//
// JSG_RESOURCE_TYPE(MyType) {
// JSG_WILDCARD_PROPERTY(getWildcard);
// }
// };
#define JSG_WILDCARD_PROPERTY(method) \
do { \
registry.template registerNamedIntercept<Self>(); \
registry.template registerWildcardProperty<Self, decltype(&Self::method), &Self::method>(); \
} while (false)

// Use inside a JSG_RESOURCE_TYPE block to declare that this type should be considered a "root" for
Expand Down
21 changes: 0 additions & 21 deletions src/workerd/jsg/jsvalue.h
Original file line number Diff line number Diff line change
Expand Up @@ -554,25 +554,4 @@ class JsMessage final {
v8::Local<v8::Message> inner;
};

inline kj::Maybe<JsValue>
NamedIntercept::getNamed(Lock&, kj::StringPtr) {
return kj::none;
}

inline kj::Maybe<NamedIntercept::Attribute>
NamedIntercept::queryNamed(Lock& js, kj::StringPtr name) {
// By default, we currently only support read only properties.
auto list = listNamed(js);
for (auto& item : list) {
if (item == name) return jsg::NamedIntercept::READ_ONLY_ATTRIBUTE;
}
return kj::none;
}

inline kj::Array<kj::String>
NamedIntercept::listNamed(Lock&) {
return kj::Array<kj::String>();
}


} // namespace workerd::jsg
3 changes: 0 additions & 3 deletions src/workerd/jsg/resource.c++
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ namespace workerd::jsg {

// TODO(cleanup): Factor out toObject(), getInterned() into some sort of v8 tools module?

const NamedIntercept::Attribute NamedIntercept::READ_ONLY_ATTRIBUTE =
NamedIntercept::Attribute::READ_ONLY | NamedIntercept::Attribute::DONT_DELETE;

void exposeGlobalScopeType(v8::Isolate* isolate, v8::Local<v8::Context> context) {
auto global = context->Global();

Expand Down
Loading
Loading