Merge pull request #1991 from lightpanda-io/v8_signature

Set v8::Signature on FunctionTemplates

.github/actions/install/action.yml

@@ -13,7 +13,7 @@ inputs:
   zig-v8:
     description: 'zig v8 version to install'
     required: false
-    default: 'v0.3.4'
+    default: 'v0.3.6'
   v8:
     description: 'v8 version to install'
     required: false

Dockerfile

@@ -3,7 +3,7 @@ FROM debian:stable-slim
 ARG MINISIG=0.12
 ARG ZIG_MINISIG=RWSGOq2NVecA2UPNdBUZykf1CCb147pkmdtYxgb3Ti+JO/wCYvhbAb/U
 ARG V8=14.0.365.4
-ARG ZIG_V8=v0.3.4
+ARG ZIG_V8=v0.3.6
 ARG TARGETPLATFORM
 
 RUN apt-get update -yq && \

build.zig.zon

@@ -5,8 +5,8 @@
     .minimum_zig_version = "0.15.2",
     .dependencies = .{
         .v8 = .{
-            .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.3.4.tar.gz",
-            .hash = "v8-0.0.0-xddH6_F3BAAiFvKY6R1H-gkuQlk19BkDQ0--uZuTrSup",
+            .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.3.6.tar.gz",
+            .hash = "v8-0.0.0-xddH6yl-BAALKs8rYDIjqpnDRQAI5sKCFX5-z7WUn7jx",
         },
         // .v8 = .{ .path = "../zig-v8-fork" },
         .brotli = .{

src/browser/js/Caller.zig

@@ -505,6 +505,7 @@ pub const Function = struct {
     pub const Opts = struct {
         noop: bool = false,
         static: bool = false,
+        deletable: bool = true,
         dom_exception: bool = false,
         as_typed_array: bool = false,
         null_as_undefined: bool = false,

src/browser/js/Snapshot.zig

@@ -137,7 +137,7 @@ pub fn create() !Snapshot {
         defer v8.v8__HandleScope__DESTRUCT(&handle_scope);
 
         // Create templates (constructors only) FIRST
-        var templates: [JsApis.len]*v8.FunctionTemplate = undefined;
+        var templates: [JsApis.len]*const v8.FunctionTemplate = undefined;
         inline for (JsApis, 0..) |JsApi, i| {
             @setEvalBranchQuota(10_000);
             templates[i] = generateConstructor(JsApi, isolate);
@@ -419,7 +419,7 @@ fn collectExternalReferences() [countExternalReferences()]isize {
 // via `new ClassName()` - but they could, for example, be created in
 // Zig and returned from a function call, which is why we need the
 // FunctionTemplate.
-fn generateConstructor(comptime JsApi: type, isolate: *v8.Isolate) *v8.FunctionTemplate {
+fn generateConstructor(comptime JsApi: type, isolate: *v8.Isolate) *const v8.FunctionTemplate {
     const callback = blk: {
         if (@hasDecl(JsApi, "constructor")) {
             break :blk JsApi.constructor.func;
@@ -429,7 +429,7 @@ fn generateConstructor(comptime JsApi: type, isolate: *v8.Isolate) *v8.FunctionT
         break :blk illegalConstructorCallback;
     };
 
-    const template = @constCast(v8.v8__FunctionTemplate__New__DEFAULT2(isolate, callback).?);
+    const template = v8.v8__FunctionTemplate__New__DEFAULT2(isolate, callback).?;
     {
         const internal_field_count = comptime countInternalFields(JsApi);
         if (internal_field_count > 0) {
@@ -482,10 +482,15 @@ pub fn countInternalFields(comptime JsApi: type) u8 {
 }
 
 // Attaches JsApi members to the prototype template (normal case)
-fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.FunctionTemplate) void {
+fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *const v8.FunctionTemplate) void {
     const instance = v8.v8__FunctionTemplate__InstanceTemplate(template);
     const prototype = v8.v8__FunctionTemplate__PrototypeTemplate(template);
 
+    // Create a signature that validates the receiver is an instance of this template.
+    // This prevents crashes when JavaScript extracts a getter/method and calls it
+    // with the wrong `this` (e.g., documentGetter.call(null)).
+    const signature = v8.v8__Signature__New(isolate, template);
+
     const declarations = @typeInfo(JsApi).@"struct".decls;
     var has_named_index_getter = false;
 
@@ -497,23 +502,47 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio
         switch (definition) {
             bridge.Accessor => {
                 const js_name = v8.v8__String__NewFromUtf8(isolate, name.ptr, v8.kNormal, @intCast(name.len));
-                const getter_callback = @constCast(v8.v8__FunctionTemplate__New__Config(isolate, &.{ .callback = value.getter }).?);
+                const getter_signature = if (value.static) null else signature;
+                const getter_callback = v8.v8__FunctionTemplate__New__Config(isolate, &.{
+                    .callback = value.getter,
+                    .signature = getter_signature,
+                }).?;
+                const setter_callback = if (value.setter) |setter|
+                    v8.v8__FunctionTemplate__New__Config(isolate, &.{
+                        .callback = setter,
+                        .signature = getter_signature,
+                    }).?
+                else
+                    null;
+
+                var attribute: v8.PropertyAttribute = 0;
                 if (value.setter == null) {
-                    if (value.static) {
-                        v8.v8__Template__SetAccessorProperty__DEFAULT(@ptrCast(template), js_name, getter_callback);
-                    } else {
-                        v8.v8__ObjectTemplate__SetAccessorProperty__DEFAULT(prototype, js_name, getter_callback);
-                    }
+                    attribute |= v8.ReadOnly;
+                }
+                if (value.deletable == false) {
+                    attribute |= v8.DontDelete;
+                }
+
+                if (value.static) {
+                    // Static accessors: use Template's SetAccessorProperty
+                    v8.v8__Template__SetAccessorProperty(@ptrCast(template), js_name, getter_callback, setter_callback, attribute);
                 } else {
-                    if (comptime IS_DEBUG) {
-                        std.debug.assert(value.static == false);
-                    }
-                    const setter_callback = @constCast(v8.v8__FunctionTemplate__New__Config(isolate, &.{ .callback = value.setter.? }).?);
-                    v8.v8__ObjectTemplate__SetAccessorProperty__DEFAULT2(prototype, js_name, getter_callback, setter_callback);
+                    v8.v8__ObjectTemplate__SetAccessorProperty__Config(prototype, &.{
+                        .key = js_name,
+                        .getter = getter_callback,
+                        .setter = setter_callback,
+                        .attribute = attribute,
+                    });
                 }
             },
             bridge.Function => {
-                const function_template = @constCast(v8.v8__FunctionTemplate__New__Config(isolate, &.{ .callback = value.func, .length = value.arity }).?);
+                // For non-static functions, use the signature to validate the receiver
+                const func_signature = if (value.static) null else signature;
+                const function_template = v8.v8__FunctionTemplate__New__Config(isolate, &.{
+                    .callback = value.func,
+                    .length = value.arity,
+                    .signature = func_signature,
+                }).?;
                 const js_name = v8.v8__String__NewFromUtf8(isolate, name.ptr, v8.kNormal, @intCast(name.len));
                 if (value.static) {
                     v8.v8__Template__Set(@ptrCast(template), js_name, @ptrCast(function_template), v8.None);
@@ -551,7 +580,7 @@ fn attachClass(comptime JsApi: type, isolate: *v8.Isolate, template: *v8.Functio
                 has_named_index_getter = true;
             },
             bridge.Iterator => {
-                const function_template = @constCast(v8.v8__FunctionTemplate__New__Config(isolate, &.{ .callback = value.func }).?);
+                const function_template = v8.v8__FunctionTemplate__New__Config(isolate, &.{ .callback = value.func }).?;
                 const js_name = if (value.async)
                     v8.v8__Symbol__GetAsyncIterator(isolate)
                 else

src/browser/js/bridge.zig

@@ -198,6 +198,7 @@ pub const Function = struct {
 
 pub const Accessor = struct {
     static: bool = false,
+    deletable: bool = true,
     cache: ?Caller.Function.Opts.Caching = null,
     getter: ?*const fn (?*const v8.FunctionCallbackInfo) callconv(.c) void = null,
     setter: ?*const fn (?*const v8.FunctionCallbackInfo) callconv(.c) void = null,
@@ -206,6 +207,7 @@ pub const Accessor = struct {
         var accessor = Accessor{
             .cache = opts.cache,
             .static = opts.static,
+            .deletable = opts.deletable,
         };
 
         if (@typeInfo(@TypeOf(getter)) != .null) {

src/browser/tests/window/window.html

@@ -262,6 +262,31 @@
   }
 </script>
 
+<script id=cached_getter_wrong_this>
+  // Test that extracting a cached property getter and calling it with wrong `this`
+  // doesn't crash (V8 internal field out of bounds). V8's Signature validation
+  // should throw "Illegal invocation" for wrong receiver types.
+  const documentGetter = Object.getOwnPropertyDescriptor(Window.prototype, 'document').get;
+
+  // Verify we get an error with wrong this values
+  let errorCount = 0;
+  const testValues = [{}, null, undefined, 42, 'string', [], () => {}];
+  for (const val of testValues) {
+    try {
+      documentGetter.call(val);
+    } catch (e) {
+      if (e.message.includes('Illegal invocation')) {
+        errorCount++;
+      }
+    }
+  }
+  // At least some should throw (null/undefined/primitives get coerced in sloppy mode)
+  testing.expectEqual(true, errorCount > 0);
+
+  // Calling with correct this should still work
+  testing.expectEqual(document, documentGetter.call(window));
+</script>
+
 <script id=unhandled_rejection>
   {
     let unhandledCalled = 0;

src/browser/webapi/Window.zig

@@ -829,7 +829,7 @@ pub const JsApi = struct {
         pub var class_id: bridge.ClassId = undefined;
     };
 
-    pub const document = bridge.accessor(Window.getDocument, null, .{ .cache = .{ .internal = 1 } });
+    pub const document = bridge.accessor(Window.getDocument, null, .{ .cache = .{ .internal = 1 }, .deletable = false });
     pub const console = bridge.accessor(Window.getConsole, null, .{ .cache = .{ .internal = 2 } });
 
     pub const top = bridge.accessor(Window.getTop, null, .{});
@@ -842,7 +842,7 @@ pub const JsApi = struct {
     pub const performance = bridge.accessor(Window.getPerformance, null, .{});
     pub const localStorage = bridge.accessor(Window.getLocalStorage, null, .{});
     pub const sessionStorage = bridge.accessor(Window.getSessionStorage, null, .{});
-    pub const location = bridge.accessor(Window.getLocation, Window.setLocation, .{});
+    pub const location = bridge.accessor(Window.getLocation, Window.setLocation, .{ .deletable = false });
     pub const history = bridge.accessor(Window.getHistory, null, .{});
     pub const navigation = bridge.accessor(Window.getNavigation, null, .{});
     pub const crypto = bridge.accessor(Window.getCrypto, null, .{});