From 70d24a9a9c1f4373c190680d46c85f9d6e0479fa Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 19 Aug 2026 17:40:47 -0700 Subject: [PATCH 01/12] Canvas: reject non-Path2D arguments instead of unwrapping them ObjectWrap::Unwrap does no type checking, so handing it an object that is not a NativeCanvasPath2D reinterprets unrelated memory as one. Four call sites reached it without establishing that: ctx.stroke(x) no check at all -- ctx.stroke("x") cast a string ctx.fill(x) gated on IsObject(), so ctx.fill({}) still got through new Path2D(x) gated on IsObject(), same hole path.addPath(x) no type check and no arity check, so addPath() also unwrapped a missing argument Add NativeCanvasPath2D::IsInstance, mirroring CanvasGradient::IsInstance, and check it before every Unwrap. It tests against the constructor kept on the native object rather than the global one, because the global is writable and the question being asked is whether the object is safe to unwrap, not whether it matches whatever Path2D currently names. fill/stroke/addPath now throw TypeError for an argument that is not a Path2D, as browsers do. The legal forms are unchanged: fill() and stroke() with no argument, an explicit undefined (which selects the no-argument overload), a fill rule string, and a Path2D. Per the (Path2D or DOMString) union, new Path2D(x) converts a non-Path2D to a string and parses it as path data rather than throwing. --- .../JavaScript/dist/tests.javaScript.all.js | 54 +++++++++++++++++++ .../JavaScript/src/tests.javaScript.all.ts | 54 +++++++++++++++++++ Polyfills/Canvas/Source/Context.cpp | 32 +++++++++-- Polyfills/Canvas/Source/Path2D.cpp | 32 ++++++++++- Polyfills/Canvas/Source/Path2D.h | 5 ++ 5 files changed, 171 insertions(+), 6 deletions(-) diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index a5d9cda5bb..b8652eb35c 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29075,6 +29075,60 @@ describe("Canvas2D", function () { (0,chai__WEBPACK_IMPORTED_MODULE_4__.expect)(ctx.strokeStyle).to.equal("#00ff00"); }); + it("rejects a non-Path2D argument to fill and stroke", function () { + // Neither call type-checked the argument before handing it to ObjectWrap::Unwrap, + // which does no checking of its own, so an unrelated object was reinterpreted as a + // NativeCanvasPath2D. stroke() was the worse of the two: it had no gate at all, so + // even a string got there. + var ctx = createContext(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke("x");}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke({});}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(5);}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill({});}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(5);}).to.throw(TypeError); + }); + + it("still accepts the valid fill and stroke argument forms", function () { + var ctx = createContext(); + var path = new Path2D("M0 0 L10 10"); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill();}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke();}).to.not.throw(); + // undefined selects the no-argument overload rather than being a bad Path2D. + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(undefined);}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(undefined);}).to.not.throw(); + // fill() also takes a fill rule string. + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill("evenodd");}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path);}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path, "nonzero");}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(path);}).to.not.throw(); + }); + + it("rejects a non-Path2D argument to Path2D.addPath", function () { + // addPath had neither a type check nor an arity check, so addPath() unwrapped a + // missing argument and addPath("x") unwrapped a string. + var path = new Path2D(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath();}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath("x");}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath({});}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath(new Path2D("M0 0 L5 5"));}).to.not.throw(); + }); + + it("treats a non-Path2D Path2D() argument as path data", function () { + // The constructor routed on IsObject(), so any object was unwrapped as a Path2D. + // Per the (Path2D or DOMString) union a non-Path2D is stringified instead. + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {new Path2D({});}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {new Path2D(5);}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {new Path2D();}).to.not.throw(); + // A copy of a real Path2D still copies, and an object that stringifies to path + // data is still parsed as such. + var source = new Path2D("M0 0 L10 10"); + var ctx = createContext(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(new Path2D(source));}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () { + ctx.fill(new Path2D({ toString: function toString() {return "M0 0 L10 10";} })); + }).to.not.throw(); + }); + it("accepts a CanvasGradient as fillStyle", function () { var ctx = createContext(); var gradient = ctx.createLinearGradient(0, 0, 64, 64); diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index d861bf035d..a825b94e40 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -573,6 +573,60 @@ describe("Canvas2D", function () { expect(ctx.strokeStyle).to.equal("#00ff00"); }); + it("rejects a non-Path2D argument to fill and stroke", function () { + // Neither call type-checked the argument before handing it to ObjectWrap::Unwrap, + // which does no checking of its own, so an unrelated object was reinterpreted as a + // NativeCanvasPath2D. stroke() was the worse of the two: it had no gate at all, so + // even a string got there. + const ctx = createContext(); + expect(function () { ctx.stroke("x"); }).to.throw(TypeError); + expect(function () { ctx.stroke({}); }).to.throw(TypeError); + expect(function () { ctx.stroke(5); }).to.throw(TypeError); + expect(function () { ctx.fill({}); }).to.throw(TypeError); + expect(function () { ctx.fill(5); }).to.throw(TypeError); + }); + + it("still accepts the valid fill and stroke argument forms", function () { + const ctx = createContext(); + const path = new Path2D("M0 0 L10 10"); + expect(function () { ctx.fill(); }).to.not.throw(); + expect(function () { ctx.stroke(); }).to.not.throw(); + // undefined selects the no-argument overload rather than being a bad Path2D. + expect(function () { ctx.fill(undefined); }).to.not.throw(); + expect(function () { ctx.stroke(undefined); }).to.not.throw(); + // fill() also takes a fill rule string. + expect(function () { ctx.fill("evenodd"); }).to.not.throw(); + expect(function () { ctx.fill(path); }).to.not.throw(); + expect(function () { ctx.fill(path, "nonzero"); }).to.not.throw(); + expect(function () { ctx.stroke(path); }).to.not.throw(); + }); + + it("rejects a non-Path2D argument to Path2D.addPath", function () { + // addPath had neither a type check nor an arity check, so addPath() unwrapped a + // missing argument and addPath("x") unwrapped a string. + const path = new Path2D(); + expect(function () { path.addPath(); }).to.throw(TypeError); + expect(function () { path.addPath("x"); }).to.throw(TypeError); + expect(function () { path.addPath({}); }).to.throw(TypeError); + expect(function () { path.addPath(new Path2D("M0 0 L5 5")); }).to.not.throw(); + }); + + it("treats a non-Path2D Path2D() argument as path data", function () { + // The constructor routed on IsObject(), so any object was unwrapped as a Path2D. + // Per the (Path2D or DOMString) union a non-Path2D is stringified instead. + expect(function () { new Path2D({}); }).to.not.throw(); + expect(function () { new Path2D(5); }).to.not.throw(); + expect(function () { new Path2D(); }).to.not.throw(); + // A copy of a real Path2D still copies, and an object that stringifies to path + // data is still parsed as such. + const source = new Path2D("M0 0 L10 10"); + const ctx = createContext(); + expect(function () { ctx.fill(new Path2D(source)); }).to.not.throw(); + expect(function () { + ctx.fill(new Path2D({ toString: function () { return "M0 0 L10 10"; } })); + }).to.not.throw(); + }); + it("accepts a CanvasGradient as fillStyle", function () { const ctx = createContext(); const gradient = ctx.createLinearGradient(0, 0, 64, 64); diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index b26d762f51..9a02e1f234 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -345,10 +345,22 @@ namespace Babylon::Polyfills::Internal { SetFilterStack(); - const NativeCanvasPath2D* path = info.Length() >= 1 && info[0].IsObject() - ? NativeCanvasPath2D::Unwrap(info[0].As()) - : nullptr; // TODO: handle fillRule: nonzero, evenodd + // fill() takes either a fillRule string or a Path2D (optionally followed by one). + // Anything else is rejected rather than unwrapped: Unwrap does no type checking, so + // an arbitrary object would be reinterpreted as a NativeCanvasPath2D. + const NativeCanvasPath2D* path = nullptr; + if (info.Length() >= 1 && !info[0].IsUndefined()) + { + if (NativeCanvasPath2D::IsInstance(info.Env(), info[0])) + { + path = NativeCanvasPath2D::Unwrap(info[0].As()); + } + else if (!info[0].IsString()) + { + throw Napi::TypeError::New(info.Env(), "Context2D.fill: the first argument is neither a Path2D nor a fill rule."); + } + } // draw Path2D if exists if (path != nullptr) @@ -654,8 +666,20 @@ namespace Babylon::Polyfills::Internal void Context::Stroke(const Napi::CallbackInfo& info) { + // stroke() takes nothing or a Path2D. The argument used to be unwrapped with no type + // check at all, so ctx.stroke("x") cast a string straight into Unwrap. + const NativeCanvasPath2D* path = nullptr; + if (info.Length() >= 1 && !info[0].IsUndefined()) + { + if (!NativeCanvasPath2D::IsInstance(info.Env(), info[0])) + { + throw Napi::TypeError::New(info.Env(), "Context2D.stroke: the first argument is not a Path2D."); + } + + path = NativeCanvasPath2D::Unwrap(info[0].As()); + } + // draw Path2D if exists - const NativeCanvasPath2D* path = info.Length() == 1 ? NativeCanvasPath2D::Unwrap(info[0].As()) : nullptr; if (path != nullptr) { PlayPath2D(path); diff --git a/Polyfills/Canvas/Source/Path2D.cpp b/Polyfills/Canvas/Source/Path2D.cpp index 2060e76ebe..cfd5531195 100644 --- a/Polyfills/Canvas/Source/Path2D.cpp +++ b/Polyfills/Canvas/Source/Path2D.cpp @@ -58,14 +58,34 @@ namespace Babylon::Polyfills::Internal global.Set(JS_PATH2D_CONSTRUCTOR_NAME, func); } + bool NativeCanvasPath2D::IsInstance(Napi::Env env, const Napi::Value& value) + { + if (!value.IsObject()) + { + return false; + } + + // Tested against the constructor kept on the native object rather than the global one: + // the global is writable, so script can replace it, and this has to answer whether the + // object is safe to Unwrap, not whether it matches whatever Path2D currently names. + const auto constructor = JsRuntime::NativeObject::GetFromJavaScript(env).Get(JS_PATH2D_CONSTRUCTOR_NAME); + return constructor.IsFunction() && value.As().InstanceOf(constructor.As()); + } + NativeCanvasPath2D::NativeCanvasPath2D(const Napi::CallbackInfo& info) : Napi::ObjectWrap{info} , m_commands{std::deque()} { - const NativeCanvasPath2D* path = info.Length() == 1 && info[0].IsObject() + // new Path2D(path) copies another Path2D; new Path2D(d) parses SVG path data. The + // argument used to be routed by IsObject(), so any object at all was fed to Unwrap and + // reinterpreted as a NativeCanvasPath2D. Per the (Path2D or DOMString) union, a value + // that is not a Path2D is converted to a string instead. + const NativeCanvasPath2D* path = info.Length() >= 1 && NativeCanvasPath2D::IsInstance(info.Env(), info[0]) ? NativeCanvasPath2D::Unwrap(info[0].As()) : nullptr; - const std::string d = info.Length() == 1 && info[0].IsString() ? info[0].As().Utf8Value() : ""; + const std::string d = path == nullptr && info.Length() >= 1 && !info[0].IsUndefined() + ? info[0].ToString().Utf8Value() + : ""; if (path != nullptr) { @@ -146,6 +166,14 @@ namespace Babylon::Polyfills::Internal void NativeCanvasPath2D::AddPath(const Napi::CallbackInfo& info) { + // addPath requires a Path2D. There was no type check and no arity check, so + // addPath() read a missing argument and addPath("x") cast a string, both of + // which reached Unwrap. + if (info.Length() < 1 || !NativeCanvasPath2D::IsInstance(info.Env(), info[0])) + { + throw Napi::TypeError::New(info.Env(), "Path2D.addPath: the first argument is not a Path2D."); + } + const NativeCanvasPath2D* path = NativeCanvasPath2D::Unwrap(info[0].As()); // optional transform arg diff --git a/Polyfills/Canvas/Source/Path2D.h b/Polyfills/Canvas/Source/Path2D.h index 9f00bb558e..acdeac2f3b 100644 --- a/Polyfills/Canvas/Source/Path2D.h +++ b/Polyfills/Canvas/Source/Path2D.h @@ -65,6 +65,11 @@ namespace Babylon::Polyfills::Internal public: static void Initialize(Napi::Env env); + // True only for objects actually produced by this class's constructor. Callers must + // check before Unwrap: ObjectWrap::Unwrap does no type checking of its own, so handing + // it any other object reinterprets unrelated memory as a NativeCanvasPath2D. + static bool IsInstance(Napi::Env env, const Napi::Value& value); + explicit NativeCanvasPath2D(const Napi::CallbackInfo& info); // virtual ~NativeCanvasPath2D(); // TODO: destructor? empty queue? From a51a2a101f8dfd4b53a51bc4b893b9ec59283cb3 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Wed, 19 Aug 2026 18:18:35 -0700 Subject: [PATCH 02/12] Do not assert the thrown error type, and split addPath's arity check A C++ Napi::TypeError surfaces as a JS TypeError on Chakra and V8 but as "InternalError: Uncaught C++ exception" on the QuickJS Node-API port, so asserting the constructor fails the Ubuntu_Clang_QuickJS job. Assert only that the call throws, which is what every other throw test in this suite already does. Also split addPath's arity check from its type check so a missing argument no longer reports that the first argument has the wrong type. --- .../JavaScript/dist/tests.javaScript.all.js | 22 +++++++++++-------- .../JavaScript/src/tests.javaScript.all.ts | 22 +++++++++++-------- Polyfills/Canvas/Source/Path2D.cpp | 7 +++++- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index b8652eb35c..5e3a141edc 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29080,12 +29080,15 @@ describe("Canvas2D", function () { // which does no checking of its own, so an unrelated object was reinterpreted as a // NativeCanvasPath2D. stroke() was the worse of the two: it had no gate at all, so // even a string got there. + // The thrown type is deliberately not asserted: a C++ Napi::TypeError surfaces as a + // JS TypeError on some engines and as an InternalError on the QuickJS Node-API port, + // so only the fact that it throws is portable. Every other throw test here does the same. var ctx = createContext(); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke("x");}).to.throw(TypeError); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke({});}).to.throw(TypeError); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(5);}).to.throw(TypeError); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill({});}).to.throw(TypeError); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(5);}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke("x");}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke({});}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(5);}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill({});}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(5);}).to.throw(); }); it("still accepts the valid fill and stroke argument forms", function () { @@ -29105,11 +29108,12 @@ describe("Canvas2D", function () { it("rejects a non-Path2D argument to Path2D.addPath", function () { // addPath had neither a type check nor an arity check, so addPath() unwrapped a - // missing argument and addPath("x") unwrapped a string. + // missing argument and addPath("x") unwrapped a string. See above for why the + // thrown type is not asserted. var path = new Path2D(); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath();}).to.throw(TypeError); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath("x");}).to.throw(TypeError); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath({});}).to.throw(TypeError); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath();}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath("x");}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath({});}).to.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath(new Path2D("M0 0 L5 5"));}).to.not.throw(); }); diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index a825b94e40..dd7629d2a3 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -578,12 +578,15 @@ describe("Canvas2D", function () { // which does no checking of its own, so an unrelated object was reinterpreted as a // NativeCanvasPath2D. stroke() was the worse of the two: it had no gate at all, so // even a string got there. + // The thrown type is deliberately not asserted: a C++ Napi::TypeError surfaces as a + // JS TypeError on some engines and as an InternalError on the QuickJS Node-API port, + // so only the fact that it throws is portable. Every other throw test here does the same. const ctx = createContext(); - expect(function () { ctx.stroke("x"); }).to.throw(TypeError); - expect(function () { ctx.stroke({}); }).to.throw(TypeError); - expect(function () { ctx.stroke(5); }).to.throw(TypeError); - expect(function () { ctx.fill({}); }).to.throw(TypeError); - expect(function () { ctx.fill(5); }).to.throw(TypeError); + expect(function () { ctx.stroke("x"); }).to.throw(); + expect(function () { ctx.stroke({}); }).to.throw(); + expect(function () { ctx.stroke(5); }).to.throw(); + expect(function () { ctx.fill({}); }).to.throw(); + expect(function () { ctx.fill(5); }).to.throw(); }); it("still accepts the valid fill and stroke argument forms", function () { @@ -603,11 +606,12 @@ describe("Canvas2D", function () { it("rejects a non-Path2D argument to Path2D.addPath", function () { // addPath had neither a type check nor an arity check, so addPath() unwrapped a - // missing argument and addPath("x") unwrapped a string. + // missing argument and addPath("x") unwrapped a string. See above for why the + // thrown type is not asserted. const path = new Path2D(); - expect(function () { path.addPath(); }).to.throw(TypeError); - expect(function () { path.addPath("x"); }).to.throw(TypeError); - expect(function () { path.addPath({}); }).to.throw(TypeError); + expect(function () { path.addPath(); }).to.throw(); + expect(function () { path.addPath("x"); }).to.throw(); + expect(function () { path.addPath({}); }).to.throw(); expect(function () { path.addPath(new Path2D("M0 0 L5 5")); }).to.not.throw(); }); diff --git a/Polyfills/Canvas/Source/Path2D.cpp b/Polyfills/Canvas/Source/Path2D.cpp index cfd5531195..06e764e390 100644 --- a/Polyfills/Canvas/Source/Path2D.cpp +++ b/Polyfills/Canvas/Source/Path2D.cpp @@ -169,7 +169,12 @@ namespace Babylon::Polyfills::Internal // addPath requires a Path2D. There was no type check and no arity check, so // addPath() read a missing argument and addPath("x") cast a string, both of // which reached Unwrap. - if (info.Length() < 1 || !NativeCanvasPath2D::IsInstance(info.Env(), info[0])) + if (info.Length() < 1) + { + throw Napi::TypeError::New(info.Env(), "Path2D.addPath: requires at least 1 argument (path)."); + } + + if (!NativeCanvasPath2D::IsInstance(info.Env(), info[0])) { throw Napi::TypeError::New(info.Env(), "Path2D.addPath: the first argument is not a Path2D."); } From a408eab9504054b413671a4e2dec19956a1d37bb Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Thu, 20 Aug 2026 09:19:51 -0700 Subject: [PATCH 03/12] Make the Path2D and gradient type checks unforgeable The IsInstance gates added earlier tested `instanceof` against the stored constructor. That only walks the prototype chain, and a prototype is assignable from script, so the check they were meant to make was still bypassable: Object.setPrototypeOf(gradient, Path2D.prototype); ctx.fill(gradient); // unwraps a CanvasGradient as a NativeCanvasPath2D Both directions crash with an access violation, confirmed on Win32 D3D11: the Path2D case through fill/stroke/addPath, and the gradient case through `ctx.fillStyle = spoofedObject` followed by any fill. Object.create with the right prototype gets through the same way while having no native wrap behind it at all. napi_type_tag_object would be the idiomatic fix, but only the V8 port implements it -- Chakra, JavaScriptCore and QuickJS do not -- and a brand property is reachable through Object.getOwnPropertySymbols and copyable onto any object. So the authority moves into C++, where script cannot reach it: NativeInstanceRegistry records the address of every live instance, and a candidate is accepted only when its unwrapped pointer is one of them. The pointer is compared, never dereferenced, before being accepted, so a foreign wrapped object is rejected instead of misread. napi_unwrap is called directly rather than through ObjectWrap::Unwrap, which throws for an object that was never wrapped. --- .../JavaScript/dist/tests.javaScript.all.js | 46 +++++++++++ .../JavaScript/src/tests.javaScript.all.ts | 46 +++++++++++ Polyfills/Canvas/CMakeLists.txt | 1 + Polyfills/Canvas/Source/Context.cpp | 24 +++--- Polyfills/Canvas/Source/Gradient.cpp | 17 ++-- Polyfills/Canvas/Source/Gradient.h | 5 ++ .../Canvas/Source/NativeInstanceRegistry.h | 78 +++++++++++++++++++ Polyfills/Canvas/Source/Path2D.cpp | 33 ++++---- Polyfills/Canvas/Source/Path2D.h | 7 +- 9 files changed, 226 insertions(+), 31 deletions(-) create mode 100644 Polyfills/Canvas/Source/NativeInstanceRegistry.h diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index 5e3a141edc..9047289ecf 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29075,6 +29075,52 @@ describe("Canvas2D", function () { (0,chai__WEBPACK_IMPORTED_MODULE_4__.expect)(ctx.strokeStyle).to.equal("#00ff00"); }); + it("rejects a prototype-spoofed object in place of a Path2D", function () { + // instanceof only walks the prototype chain, and a prototype is assignable, so an + // InstanceOf gate accepted any object wearing Path2D.prototype and then unwrapped it. + // Handing fill() a CanvasGradient this way was an access violation, not a wrong answer. + // Object.create(Path2D.prototype) is the same hole with no native wrap behind it at all. + var ctx = createContext(); + var spoofedGradient = ctx.createLinearGradient(0, 0, 10, 10); + Object.setPrototypeOf(spoofedGradient, Path2D.prototype); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(spoofedGradient instanceof Path2D).to.equal(true); + + var bare = Object.create(Path2D.prototype); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(bare instanceof Path2D).to.equal(true); + + var realPath = new Path2D();var _loop = function _loop() + {var impostor = _arr[_i]; + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(impostor);}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(impostor);}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {realPath.addPath(impostor);}).to.throw(); + // The Path2D() argument is a (Path2D or DOMString) union, so a non-Path2D is string + // data rather than an error. It must not be unwrapped on the way there. + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {new Path2D(impostor);}).to.not.throw(); + };for (var _i = 0, _arr = [spoofedGradient, bare]; _i < _arr.length; _i++) {_loop();} + }); + + it("ignores a prototype-spoofed object assigned to fillStyle or strokeStyle", function () { + // Same defect on the gradient side: the assignment gate accepted anything wearing the + // gradient prototype and stored it, and the next fill unwrapped it as a CanvasGradient. + var ctx = createContext(); + var gradient = ctx.createLinearGradient(0, 0, 10, 10); + var spoofedPath = new Path2D(); + Object.setPrototypeOf(spoofedPath, Object.getPrototypeOf(gradient)); + + ctx.fillStyle = "#ff0000"; + ctx.strokeStyle = "#00ff00"; + ctx.fillStyle = spoofedPath; + ctx.strokeStyle = spoofedPath; + + // Per spec an unusable assignment leaves the previous value in place. + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(ctx.fillStyle).to.equal("#ff0000"); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(ctx.strokeStyle).to.equal("#00ff00"); + + // The drawing path must stay usable rather than unwrapping the impostor. + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fillRect(0, 0, 10, 10);}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.strokeRect(0, 0, 10, 10);}).to.not.throw(); + }); + it("rejects a non-Path2D argument to fill and stroke", function () { // Neither call type-checked the argument before handing it to ObjectWrap::Unwrap, // which does no checking of its own, so an unrelated object was reinterpreted as a diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index dd7629d2a3..4ff8411b4d 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -573,6 +573,52 @@ describe("Canvas2D", function () { expect(ctx.strokeStyle).to.equal("#00ff00"); }); + it("rejects a prototype-spoofed object in place of a Path2D", function () { + // instanceof only walks the prototype chain, and a prototype is assignable, so an + // InstanceOf gate accepted any object wearing Path2D.prototype and then unwrapped it. + // Handing fill() a CanvasGradient this way was an access violation, not a wrong answer. + // Object.create(Path2D.prototype) is the same hole with no native wrap behind it at all. + const ctx = createContext(); + const spoofedGradient: any = ctx.createLinearGradient(0, 0, 10, 10); + Object.setPrototypeOf(spoofedGradient, Path2D.prototype); + expect(spoofedGradient instanceof Path2D).to.equal(true); + + const bare: any = Object.create(Path2D.prototype); + expect(bare instanceof Path2D).to.equal(true); + + const realPath = new Path2D(); + for (const impostor of [spoofedGradient, bare]) { + expect(function () { ctx.fill(impostor); }).to.throw(); + expect(function () { ctx.stroke(impostor); }).to.throw(); + expect(function () { realPath.addPath(impostor); }).to.throw(); + // The Path2D() argument is a (Path2D or DOMString) union, so a non-Path2D is string + // data rather than an error. It must not be unwrapped on the way there. + expect(function () { new Path2D(impostor); }).to.not.throw(); + } + }); + + it("ignores a prototype-spoofed object assigned to fillStyle or strokeStyle", function () { + // Same defect on the gradient side: the assignment gate accepted anything wearing the + // gradient prototype and stored it, and the next fill unwrapped it as a CanvasGradient. + const ctx = createContext(); + const gradient = ctx.createLinearGradient(0, 0, 10, 10); + const spoofedPath: any = new Path2D(); + Object.setPrototypeOf(spoofedPath, Object.getPrototypeOf(gradient)); + + ctx.fillStyle = "#ff0000"; + ctx.strokeStyle = "#00ff00"; + ctx.fillStyle = spoofedPath; + ctx.strokeStyle = spoofedPath; + + // Per spec an unusable assignment leaves the previous value in place. + expect(ctx.fillStyle).to.equal("#ff0000"); + expect(ctx.strokeStyle).to.equal("#00ff00"); + + // The drawing path must stay usable rather than unwrapping the impostor. + expect(function () { ctx.fillRect(0, 0, 10, 10); }).to.not.throw(); + expect(function () { ctx.strokeRect(0, 0, 10, 10); }).to.not.throw(); + }); + it("rejects a non-Path2D argument to fill and stroke", function () { // Neither call type-checked the argument before handing it to ObjectWrap::Unwrap, // which does no checking of its own, so an unrelated object was reinterpreted as a diff --git a/Polyfills/Canvas/CMakeLists.txt b/Polyfills/Canvas/CMakeLists.txt index 3103b004cf..22affaf88d 100644 --- a/Polyfills/Canvas/CMakeLists.txt +++ b/Polyfills/Canvas/CMakeLists.txt @@ -22,6 +22,7 @@ set(SOURCES "Source/Context.h" "Source/MeasureText.cpp" "Source/MeasureText.h" + "Source/NativeInstanceRegistry.h" "Source/Gradient.cpp" "Source/Gradient.h" "Source/Font.cpp" diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 9a02e1f234..7e3e1ef86c 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -201,7 +201,11 @@ namespace Babylon::Polyfills::Internal } else if (std::holds_alternative(m_state.fillStyle)) { - CanvasGradient* gradient = CanvasGradient::Unwrap(std::get(m_state.fillStyle)->Value()); + CanvasGradient* gradient = CanvasGradient::TryUnwrap(info.Env(), std::get(m_state.fillStyle)->Value()); + if (gradient == nullptr) + { + throw Napi::Error::New(info.Env(), "Fillstyle is not a color string or a gradient."); + } nvgFillPaint(*m_nvg, gradient->Paint()); } else @@ -223,7 +227,11 @@ namespace Babylon::Polyfills::Internal } else if (std::holds_alternative(m_state.strokeStyle)) { - CanvasGradient* gradient = CanvasGradient::Unwrap(std::get(m_state.strokeStyle)->Value()); + CanvasGradient* gradient = CanvasGradient::TryUnwrap(info.Env(), std::get(m_state.strokeStyle)->Value()); + if (gradient == nullptr) + { + throw Napi::Error::New(info.Env(), "Strokestyle is not a color string or a gradient."); + } nvgStrokePaint(*m_nvg, gradient->Paint()); } else @@ -352,11 +360,8 @@ namespace Babylon::Polyfills::Internal const NativeCanvasPath2D* path = nullptr; if (info.Length() >= 1 && !info[0].IsUndefined()) { - if (NativeCanvasPath2D::IsInstance(info.Env(), info[0])) - { - path = NativeCanvasPath2D::Unwrap(info[0].As()); - } - else if (!info[0].IsString()) + path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); + if (path == nullptr && !info[0].IsString()) { throw Napi::TypeError::New(info.Env(), "Context2D.fill: the first argument is neither a Path2D nor a fill rule."); } @@ -671,12 +676,11 @@ namespace Babylon::Polyfills::Internal const NativeCanvasPath2D* path = nullptr; if (info.Length() >= 1 && !info[0].IsUndefined()) { - if (!NativeCanvasPath2D::IsInstance(info.Env(), info[0])) + path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); + if (path == nullptr) { throw Napi::TypeError::New(info.Env(), "Context2D.stroke: the first argument is not a Path2D."); } - - path = NativeCanvasPath2D::Unwrap(info[0].As()); } // draw Path2D if exists diff --git a/Polyfills/Canvas/Source/Gradient.cpp b/Polyfills/Canvas/Source/Gradient.cpp index e237157a08..a52ff271e4 100644 --- a/Polyfills/Canvas/Source/Gradient.cpp +++ b/Polyfills/Canvas/Source/Gradient.cpp @@ -3,6 +3,7 @@ #include "Context.h" #include "Gradient.h" #include "Colors.h" +#include "NativeInstanceRegistry.h" #include #include @@ -120,13 +121,12 @@ namespace Babylon::Polyfills::Internal bool CanvasGradient::IsInstance(Napi::Env env, const Napi::Value& value) { - if (!value.IsObject()) - { - return false; - } + return TryUnwrap(env, value) != nullptr; + } - const auto constructor = JsRuntime::NativeObject::GetFromJavaScript(env).Get(JS_CANVAS_GRADIENT_CONSTRUCTOR_NAME); - return constructor.IsFunction() && value.As().InstanceOf(constructor.As()); + CanvasGradient* CanvasGradient::TryUnwrap(Napi::Env env, const Napi::Value& value) + { + return NativeInstanceRegistry::TryUnwrap(env, value); } Napi::Object CanvasGradient::CreateLinear(Napi::Env env, const std::shared_ptr& context, float x0, float y0, float x1, float y1) @@ -163,10 +163,15 @@ namespace Babylon::Polyfills::Internal r0 = info[4].As().FloatValue(); r1 = info[5].As().FloatValue(); } + + // Registered last: a constructor that throws never reaches the destructor, which + // would leave a dangling address in the registry. + NativeInstanceRegistry::Add(this); } CanvasGradient::~CanvasGradient() { + NativeInstanceRegistry::Remove(this); Dispose(); } diff --git a/Polyfills/Canvas/Source/Gradient.h b/Polyfills/Canvas/Source/Gradient.h index 913a257b5d..fb3d427b7c 100644 --- a/Polyfills/Canvas/Source/Gradient.h +++ b/Polyfills/Canvas/Source/Gradient.h @@ -22,6 +22,11 @@ namespace Babylon::Polyfills::Internal // script and would otherwise unwrap an object that was never wrapped. static bool IsInstance(Napi::Env env, const Napi::Value& value); + // Returns the wrapped gradient, or nullptr when `value` is not one. Prefer this to + // IsInstance + Unwrap; see NativeInstanceRegistry for why neither `instanceof` nor a + // brand property can answer this safely. + static CanvasGradient* TryUnwrap(Napi::Env env, const Napi::Value& value); + explicit CanvasGradient(const Napi::CallbackInfo& info); virtual ~CanvasGradient(); diff --git a/Polyfills/Canvas/Source/NativeInstanceRegistry.h b/Polyfills/Canvas/Source/NativeInstanceRegistry.h new file mode 100644 index 0000000000..e6e38369ec --- /dev/null +++ b/Polyfills/Canvas/Source/NativeInstanceRegistry.h @@ -0,0 +1,78 @@ +#pragma once + +#include +#include +#include + +namespace Babylon::Polyfills::Internal +{ + // Answers "is this JS object wrapping a T", for types whose native pointer is handed to + // napi_unwrap. It exists because the obvious answers are all forgeable from script: + // + // - `instanceof` walks the prototype chain, and a prototype is assignable. After + // Object.setPrototypeOf(gradient, Path2D.prototype) a CanvasGradient passes an + // InstanceOf test against Path2D, and unwrapping it reinterprets the gradient's + // native pointer as a path. That is an access violation, not a wrong answer. + // - Object.create(Path2D.prototype) passes the same test while having no native wrap + // at all. + // - A brand property, even under a symbol, is reachable via + // Object.getOwnPropertySymbols and can be copied onto any object. + // - napi_type_tag_object would be the idiomatic answer, but only the V8 port + // implements it; the Chakra, JavaScriptCore and QuickJS ports do not. + // + // So the authority is kept here in C++, where script cannot reach it: every live instance + // records its own address, and a candidate is accepted only if its unwrapped pointer is + // one of them. The pointer is compared, never dereferenced, before it is accepted, so a + // foreign wrapped object is rejected rather than misread. + template + class NativeInstanceRegistry + { + public: + static void Add(const T* instance) + { + const std::scoped_lock lock{Mutex()}; + Instances().insert(instance); + } + + static void Remove(const T* instance) + { + const std::scoped_lock lock{Mutex()}; + Instances().erase(instance); + } + + // Returns the wrapped instance, or nullptr when `value` is not one. napi_unwrap is + // called directly rather than through ObjectWrap::Unwrap because the latter throws for + // an object that was never wrapped, and this has to answer "no" for any value at all. + static T* TryUnwrap(Napi::Env env, const Napi::Value& value) + { + if (!value.IsObject()) + { + return nullptr; + } + + void* unwrapped{}; + if (napi_unwrap(env, value, &unwrapped) != napi_ok || unwrapped == nullptr) + { + return nullptr; + } + + const std::scoped_lock lock{Mutex()}; + return Instances().count(unwrapped) != 0 ? static_cast(unwrapped) : nullptr; + } + + private: + // Function-local statics so this header needs no out-of-line definitions. The mutex + // covers hosts running more than one JS environment, whose instances share these. + static std::mutex& Mutex() + { + static std::mutex mutex{}; + return mutex; + } + + static std::unordered_set& Instances() + { + static std::unordered_set instances{}; + return instances; + } + }; +} diff --git a/Polyfills/Canvas/Source/Path2D.cpp b/Polyfills/Canvas/Source/Path2D.cpp index 06e764e390..4d9dfa2b08 100644 --- a/Polyfills/Canvas/Source/Path2D.cpp +++ b/Polyfills/Canvas/Source/Path2D.cpp @@ -2,6 +2,7 @@ #include #include #include "Canvas.h" +#include "NativeInstanceRegistry.h" #include "Path2D.h" #include @@ -60,16 +61,12 @@ namespace Babylon::Polyfills::Internal bool NativeCanvasPath2D::IsInstance(Napi::Env env, const Napi::Value& value) { - if (!value.IsObject()) - { - return false; - } + return TryUnwrap(env, value) != nullptr; + } - // Tested against the constructor kept on the native object rather than the global one: - // the global is writable, so script can replace it, and this has to answer whether the - // object is safe to Unwrap, not whether it matches whatever Path2D currently names. - const auto constructor = JsRuntime::NativeObject::GetFromJavaScript(env).Get(JS_PATH2D_CONSTRUCTOR_NAME); - return constructor.IsFunction() && value.As().InstanceOf(constructor.As()); + NativeCanvasPath2D* NativeCanvasPath2D::TryUnwrap(Napi::Env env, const Napi::Value& value) + { + return NativeInstanceRegistry::TryUnwrap(env, value); } NativeCanvasPath2D::NativeCanvasPath2D(const Napi::CallbackInfo& info) @@ -80,8 +77,8 @@ namespace Babylon::Polyfills::Internal // argument used to be routed by IsObject(), so any object at all was fed to Unwrap and // reinterpreted as a NativeCanvasPath2D. Per the (Path2D or DOMString) union, a value // that is not a Path2D is converted to a string instead. - const NativeCanvasPath2D* path = info.Length() >= 1 && NativeCanvasPath2D::IsInstance(info.Env(), info[0]) - ? NativeCanvasPath2D::Unwrap(info[0].As()) + const NativeCanvasPath2D* path = info.Length() >= 1 + ? NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]) : nullptr; const std::string d = path == nullptr && info.Length() >= 1 && !info[0].IsUndefined() ? info[0].ToString().Utf8Value() @@ -137,6 +134,15 @@ namespace Babylon::Polyfills::Internal nsvg__deleteParser(parser); } + + // Registered last: everything above can throw, and a constructor that throws never + // reaches the destructor, which would leave a dangling address in the registry. + NativeInstanceRegistry::Add(this); + } + + NativeCanvasPath2D::~NativeCanvasPath2D() + { + NativeInstanceRegistry::Remove(this); } typename std::deque::iterator NativeCanvasPath2D::begin() @@ -174,13 +180,12 @@ namespace Babylon::Polyfills::Internal throw Napi::TypeError::New(info.Env(), "Path2D.addPath: requires at least 1 argument (path)."); } - if (!NativeCanvasPath2D::IsInstance(info.Env(), info[0])) + NativeCanvasPath2D* const path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); + if (path == nullptr) { throw Napi::TypeError::New(info.Env(), "Path2D.addPath: the first argument is not a Path2D."); } - const NativeCanvasPath2D* path = NativeCanvasPath2D::Unwrap(info[0].As()); - // optional transform arg bool xformInvReady{false}; float xformInv[6]; diff --git a/Polyfills/Canvas/Source/Path2D.h b/Polyfills/Canvas/Source/Path2D.h index acdeac2f3b..6b203e070d 100644 --- a/Polyfills/Canvas/Source/Path2D.h +++ b/Polyfills/Canvas/Source/Path2D.h @@ -70,8 +70,13 @@ namespace Babylon::Polyfills::Internal // it any other object reinterprets unrelated memory as a NativeCanvasPath2D. static bool IsInstance(Napi::Env env, const Napi::Value& value); + // Returns the wrapped path, or nullptr when `value` is not one of ours. Prefer this to + // IsInstance + Unwrap: it answers the question and hands back the pointer in one step, + // and unlike ObjectWrap::Unwrap it never throws for an object that was never wrapped. + static NativeCanvasPath2D* TryUnwrap(Napi::Env env, const Napi::Value& value); + explicit NativeCanvasPath2D(const Napi::CallbackInfo& info); - // virtual ~NativeCanvasPath2D(); // TODO: destructor? empty queue? + ~NativeCanvasPath2D(); typename std::deque::iterator begin(); typename std::deque::iterator end(); From e4a2f03509b164ae89d508d0278f852cc7b2b3a1 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Thu, 20 Aug 2026 10:36:17 -0700 Subject: [PATCH 04/12] Canvas: brand native instances instead of unwrapping unchecked objects The registry added in the previous commit still reached the instance pointer through ObjectWrap::Unwrap, which is not safe to call on an object that may never have been wrapped. Neither JsRuntimeHost port honours that contract: the V8 port dereferences internal field 0 unconditionally and access-violates, and the QuickJS port falls back to walking the prototype chain and returns some other object's pointer. Object.create(Path2D.prototype) crashed the V8 and QuickJS unit test runs for exactly this reason. Each instance now brands its own JS object with an External holding its address, and a candidate is accepted only if that address is still registered. Externals are opaque to script and the address is compared, never dereferenced, before it is accepted. Verified on Chakra, V8 and QuickJS: 21/21 gtest, 49 assertions. Reverting only the C++ change reproduces the access violation. Visual sweep 305/305. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88569c10-a7ff-4373-9a58-afa9c68b8c09 --- Polyfills/Canvas/Source/Gradient.cpp | 5 +- Polyfills/Canvas/Source/Gradient.h | 10 +-- .../Canvas/Source/NativeInstanceRegistry.h | 82 ++++++++++++------- Polyfills/Canvas/Source/Path2D.cpp | 5 +- Polyfills/Canvas/Source/Path2D.h | 9 +- 5 files changed, 63 insertions(+), 48 deletions(-) diff --git a/Polyfills/Canvas/Source/Gradient.cpp b/Polyfills/Canvas/Source/Gradient.cpp index a52ff271e4..1c0d41499c 100644 --- a/Polyfills/Canvas/Source/Gradient.cpp +++ b/Polyfills/Canvas/Source/Gradient.cpp @@ -164,9 +164,8 @@ namespace Babylon::Polyfills::Internal r1 = info[5].As().FloatValue(); } - // Registered last: a constructor that throws never reaches the destructor, which - // would leave a dangling address in the registry. - NativeInstanceRegistry::Add(this); + // Registered last: a constructor that throws never reaches the destructor. + NativeInstanceRegistry::Add(info, this); } CanvasGradient::~CanvasGradient() diff --git a/Polyfills/Canvas/Source/Gradient.h b/Polyfills/Canvas/Source/Gradient.h index fb3d427b7c..c69cd19af2 100644 --- a/Polyfills/Canvas/Source/Gradient.h +++ b/Polyfills/Canvas/Source/Gradient.h @@ -16,15 +16,11 @@ namespace Babylon::Polyfills::Internal static Napi::Object CreateLinear(Napi::Env env, const std::shared_ptr& context, float x0, float y0, float x1, float y1); static Napi::Object CreateRadial(Napi::Env env, const std::shared_ptr& context, float x0, float y0, float r0, float x1, float y1, float r1); - // True only for objects this polyfill's own constructor produced. Unwrap() is a - // napi_unwrap, which dereferences whatever the object's slot happens to hold, so - // callers must establish the type first: `ctx.strokeStyle = {}` is reachable from - // script and would otherwise unwrap an object that was never wrapped. + // True only for objects this polyfill's own constructor produced. Never use + // ObjectWrap::Unwrap on an unchecked object; see NativeInstanceRegistry. static bool IsInstance(Napi::Env env, const Napi::Value& value); - // Returns the wrapped gradient, or nullptr when `value` is not one. Prefer this to - // IsInstance + Unwrap; see NativeInstanceRegistry for why neither `instanceof` nor a - // brand property can answer this safely. + // Returns the wrapped gradient, or nullptr when `value` is not one. static CanvasGradient* TryUnwrap(Napi::Env env, const Napi::Value& value); explicit CanvasGradient(const Napi::CallbackInfo& info); diff --git a/Polyfills/Canvas/Source/NativeInstanceRegistry.h b/Polyfills/Canvas/Source/NativeInstanceRegistry.h index e6e38369ec..d0bfd179fa 100644 --- a/Polyfills/Canvas/Source/NativeInstanceRegistry.h +++ b/Polyfills/Canvas/Source/NativeInstanceRegistry.h @@ -6,30 +6,36 @@ namespace Babylon::Polyfills::Internal { - // Answers "is this JS object wrapping a T", for types whose native pointer is handed to - // napi_unwrap. It exists because the obvious answers are all forgeable from script: + // Answers "is this JS object wrapping a T", and hands back the instance. // - // - `instanceof` walks the prototype chain, and a prototype is assignable. After - // Object.setPrototypeOf(gradient, Path2D.prototype) a CanvasGradient passes an - // InstanceOf test against Path2D, and unwrapping it reinterprets the gradient's - // native pointer as a path. That is an access violation, not a wrong answer. - // - Object.create(Path2D.prototype) passes the same test while having no native wrap - // at all. - // - A brand property, even under a symbol, is reachable via - // Object.getOwnPropertySymbols and can be copied onto any object. - // - napi_type_tag_object would be the idiomatic answer, but only the V8 port - // implements it; the Chakra, JavaScriptCore and QuickJS ports do not. + // `instanceof` cannot answer it: a prototype is assignable, so + // Object.setPrototypeOf(gradient, Path2D.prototype) makes a gradient pass a Path2D test, + // and Object.create(Path2D.prototype) passes with no native object behind it at all. // - // So the authority is kept here in C++, where script cannot reach it: every live instance - // records its own address, and a candidate is accepted only if its unwrapped pointer is - // one of them. The pointer is compared, never dereferenced, before it is accepted, so a - // foreign wrapped object is rejected rather than misread. + // Neither can ObjectWrap::Unwrap. Both JsRuntimeHost Node-API ports break the contract + // that it fails for an object that was never wrapped: the V8 port dereferences internal + // field 0 unconditionally, which access-violates, and the QuickJS port falls back to + // walking the prototype chain, which returns some other object's pointer. + // + // So each instance brands its own JS object with an External holding its address, and a + // candidate is accepted only if that address is still registered here. An External is + // opaque to script, and the address is compared, never dereferenced, before it is + // accepted, so both spoofs above are rejected instead of being misread as a T. + // + // Script can still copy a brand off a real instance onto another object. That is not a + // memory-safety hole: the result is the live instance the brand came from, which script + // had to already hold. A brand left over from a collected instance is rejected, since the + // destructor unregisters the address. template class NativeInstanceRegistry { public: - static void Add(const T* instance) + // Call at the very end of the constructor: one that throws never reaches the + // destructor, which would leave a dangling address registered. + static void Add(const Napi::CallbackInfo& info, T* instance) { + info.This().As().Set(BRAND_NAME, Napi::External::New(info.Env(), instance)); + const std::scoped_lock lock{Mutex()}; Instances().insert(instance); } @@ -40,9 +46,7 @@ namespace Babylon::Polyfills::Internal Instances().erase(instance); } - // Returns the wrapped instance, or nullptr when `value` is not one. napi_unwrap is - // called directly rather than through ObjectWrap::Unwrap because the latter throws for - // an object that was never wrapped, and this has to answer "no" for any value at all. + // Returns the wrapped instance, or nullptr when `value` is not one. static T* TryUnwrap(Napi::Env env, const Napi::Value& value) { if (!value.IsObject()) @@ -50,29 +54,49 @@ namespace Babylon::Polyfills::Internal return nullptr; } - void* unwrapped{}; - if (napi_unwrap(env, value, &unwrapped) != napi_ok || unwrapped == nullptr) + // `value` is arbitrary, so the read can run a script accessor that throws. + Napi::Value brand{env.Undefined()}; + try + { + brand = value.As().Get(BRAND_NAME); + } + catch (...) + { + } + + if (env.IsExceptionPending()) + { + (void)env.GetAndClearPendingException(); + } + + if (!brand.IsExternal()) { return nullptr; } + T* const candidate = brand.As>().Data(); + const std::scoped_lock lock{Mutex()}; - return Instances().count(unwrapped) != 0 ? static_cast(unwrapped) : nullptr; + return Instances().count(candidate) != 0 ? candidate : nullptr; } private: - // Function-local statics so this header needs no out-of-line definitions. The mutex - // covers hosts running more than one JS environment, whose instances share these. + // Shared by every T: a brand read as the wrong type is still rejected, because each T + // registers into its own set. + static constexpr const char* BRAND_NAME{"__nativeInstance"}; + + // Never destroyed, so that an instance finalized during static destruction cannot + // erase itself from an already-destroyed set. static std::mutex& Mutex() { - static std::mutex mutex{}; - return mutex; + static auto* mutex{new std::mutex{}}; + return *mutex; } static std::unordered_set& Instances() { - static std::unordered_set instances{}; - return instances; + static auto* instances{new std::unordered_set{}}; + return *instances; } }; } diff --git a/Polyfills/Canvas/Source/Path2D.cpp b/Polyfills/Canvas/Source/Path2D.cpp index 4d9dfa2b08..073b7662d9 100644 --- a/Polyfills/Canvas/Source/Path2D.cpp +++ b/Polyfills/Canvas/Source/Path2D.cpp @@ -135,9 +135,8 @@ namespace Babylon::Polyfills::Internal nsvg__deleteParser(parser); } - // Registered last: everything above can throw, and a constructor that throws never - // reaches the destructor, which would leave a dangling address in the registry. - NativeInstanceRegistry::Add(this); + // Registered last: a constructor that throws never reaches the destructor. + NativeInstanceRegistry::Add(info, this); } NativeCanvasPath2D::~NativeCanvasPath2D() diff --git a/Polyfills/Canvas/Source/Path2D.h b/Polyfills/Canvas/Source/Path2D.h index 6b203e070d..0ec53c8d08 100644 --- a/Polyfills/Canvas/Source/Path2D.h +++ b/Polyfills/Canvas/Source/Path2D.h @@ -65,14 +65,11 @@ namespace Babylon::Polyfills::Internal public: static void Initialize(Napi::Env env); - // True only for objects actually produced by this class's constructor. Callers must - // check before Unwrap: ObjectWrap::Unwrap does no type checking of its own, so handing - // it any other object reinterprets unrelated memory as a NativeCanvasPath2D. + // True only for objects actually produced by this class's constructor. Never use + // ObjectWrap::Unwrap on an unchecked object; see NativeInstanceRegistry. static bool IsInstance(Napi::Env env, const Napi::Value& value); - // Returns the wrapped path, or nullptr when `value` is not one of ours. Prefer this to - // IsInstance + Unwrap: it answers the question and hands back the pointer in one step, - // and unlike ObjectWrap::Unwrap it never throws for an object that was never wrapped. + // Returns the wrapped path, or nullptr when `value` is not one of ours. static NativeCanvasPath2D* TryUnwrap(Napi::Env env, const Napi::Value& value); explicit NativeCanvasPath2D(const Napi::CallbackInfo& info); From 7adaab032c54e7596fae0770e39df7bbb8f7d436 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Thu, 27 Aug 2026 16:15:50 -0700 Subject: [PATCH 05/12] Canvas: brand Image and reject unsafe drawImage unwraps Extend the NativeInstanceRegistry brand to NativeCanvasImage so drawImage no longer ObjectWrap::Unwraps an unchecked first argument (Canvas, Path2D, or plain object), which access-violated on V8. TryUnwrap + TypeError instead. Adds a unit test that drawImage throws for non-Image sources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js | 10 ++++++++++ Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts | 10 ++++++++++ Polyfills/Canvas/Source/Context.cpp | 10 ++++++++-- Polyfills/Canvas/Source/Image.cpp | 9 +++++++++ Polyfills/Canvas/Source/Image.h | 3 +++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index 9047289ecf..c666f720a9 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29163,6 +29163,16 @@ describe("Canvas2D", function () { (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath(new Path2D("M0 0 L5 5"));}).to.not.throw(); }); + it("rejects a non-Image argument to drawImage", function () { + // drawImage used ObjectWrap::Unwrap on whatever object it was handed, so a Canvas, + // Path2D, or plain {} was an access violation rather than a TypeError. + var ctx = createContext(); + var otherCanvas = new _native.Canvas(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage({}, 0, 0);}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage(new Path2D(), 0, 0);}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage(otherCanvas, 0, 0);}).to.throw(); + }); + it("treats a non-Path2D Path2D() argument as path data", function () { // The constructor routed on IsObject(), so any object was unwrapped as a Path2D. // Per the (Path2D or DOMString) union a non-Path2D is stringified instead. diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index 4ff8411b4d..171267f918 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -661,6 +661,16 @@ describe("Canvas2D", function () { expect(function () { path.addPath(new Path2D("M0 0 L5 5")); }).to.not.throw(); }); + it("rejects a non-Image argument to drawImage", function () { + // drawImage used ObjectWrap::Unwrap on whatever object it was handed, so a Canvas, + // Path2D, or plain {} was an access violation rather than a TypeError. + const ctx = createContext(); + const otherCanvas = new _native.Canvas(); + expect(function () { ctx.drawImage({}, 0, 0); }).to.throw(); + expect(function () { ctx.drawImage(new Path2D(), 0, 0); }).to.throw(); + expect(function () { ctx.drawImage(otherCanvas, 0, 0); }).to.throw(); + }); + it("treats a non-Path2D Path2D() argument as path data", function () { // The constructor routed on IsObject(), so any object was unwrapped as a Path2D. // Per the (Path2D or DOMString) union a non-Path2D is stringified instead. diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 7e3e1ef86c..384e31249d 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -1287,7 +1287,14 @@ namespace Babylon::Polyfills::Internal NativeCanvas* const srcCanvas = canvasCtorVal.IsFunction() && imageObj.InstanceOf(canvasCtorVal.As()) ? NativeCanvas::Unwrap(imageObj) : nullptr; - const bool isImageBitmap = srcCanvas == nullptr && imageObj.Has("data") && imageObj.Get("data").IsTypedArray(); + const NativeCanvasImage* const canvasImage = srcCanvas == nullptr + ? NativeCanvasImage::TryUnwrap(info.Env(), imageObj) + : nullptr; + const bool isImageBitmap = srcCanvas == nullptr && canvasImage == nullptr && imageObj.Has("data") && imageObj.Get("data").IsTypedArray(); + if (srcCanvas == nullptr && canvasImage == nullptr && !isImageBitmap) + { + throw Napi::TypeError::New(info.Env(), "drawImage: first argument must be a Canvas, Image, or ImageBitmap-like object."); + } // Coercion can also resize the source. Do it once, before reading dimensions // or creating any graphics resources. @@ -1414,7 +1421,6 @@ namespace Babylon::Polyfills::Internal #endif } - const NativeCanvasImage* canvasImage = NativeCanvasImage::Unwrap(imageObj); const auto rectangles = ParseDrawImageRectangles(arguments, canvasImage->GetWidth(), canvasImage->GetHeight()); if (!rectangles) { diff --git a/Polyfills/Canvas/Source/Image.cpp b/Polyfills/Canvas/Source/Image.cpp index 306a5b7815..22b3472531 100644 --- a/Polyfills/Canvas/Source/Image.cpp +++ b/Polyfills/Canvas/Source/Image.cpp @@ -3,6 +3,7 @@ #include "Canvas.h" #include "Image.h" #include "Context.h" +#include "NativeInstanceRegistry.h" #include #include #include @@ -43,15 +44,23 @@ namespace Babylon::Polyfills::Internal JsRuntime::NativeObject::GetFromJavaScript(env).Set(JS_IMAGE_CONSTRUCTOR_NAME, func); } + NativeCanvasImage* NativeCanvasImage::TryUnwrap(Napi::Env env, const Napi::Value& value) + { + return NativeInstanceRegistry::TryUnwrap(env, value); + } + NativeCanvasImage::NativeCanvasImage(const Napi::CallbackInfo& info) : Napi::ObjectWrap{info} , m_runtimeScheduler{JsRuntime::GetFromJavaScript(info.Env())} , m_cancellationSource{std::make_shared()} { + // Registered last: a constructor that throws never reaches the destructor. + NativeInstanceRegistry::Add(info, this); } NativeCanvasImage::~NativeCanvasImage() { + NativeInstanceRegistry::Remove(this); Dispose(); } diff --git a/Polyfills/Canvas/Source/Image.h b/Polyfills/Canvas/Source/Image.h index bc40b60257..11ea362a78 100644 --- a/Polyfills/Canvas/Source/Image.h +++ b/Polyfills/Canvas/Source/Image.h @@ -19,6 +19,9 @@ namespace Babylon::Polyfills::Internal public: static void Initialize(Napi::Env env); + // Safe brand check — never use ObjectWrap::Unwrap on an unchecked JS value. + static NativeCanvasImage* TryUnwrap(Napi::Env env, const Napi::Value& value); + explicit NativeCanvasImage(const Napi::CallbackInfo& info); virtual ~NativeCanvasImage(); From 8d07635a844ba56c783bba7e020a3d54ffae0f89 Mon Sep 17 00:00:00 2001 From: Branimir Karadzic Date: Thu, 27 Aug 2026 16:47:20 -0700 Subject: [PATCH 06/12] Canvas: tighten branding PR (non-enumerable brand, trim comments) - Define brand with napi_default (non-enumerable) via DefineProperty - fillStyle/strokeStyle TypeError messages use JS property names - Trim long explanatory comments; keep only non-obvious notes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 60c2ec68-6de1-445d-9fc9-b699db737eae --- .../JavaScript/dist/tests.javaScript.all.js | 3 +- .../JavaScript/src/tests.javaScript.all.ts | 5 +-- Polyfills/Canvas/Source/Context.cpp | 19 ++++----- Polyfills/Canvas/Source/Gradient.cpp | 4 +- Polyfills/Canvas/Source/Image.cpp | 4 +- Polyfills/Canvas/Source/Image.h | 3 +- .../Canvas/Source/NativeInstanceRegistry.h | 42 ++++++------------- Polyfills/Canvas/Source/Path2D.cpp | 16 +++---- 8 files changed, 33 insertions(+), 63 deletions(-) diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index c666f720a9..a4c95dce43 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29164,8 +29164,7 @@ describe("Canvas2D", function () { }); it("rejects a non-Image argument to drawImage", function () { - // drawImage used ObjectWrap::Unwrap on whatever object it was handed, so a Canvas, - // Path2D, or plain {} was an access violation rather than a TypeError. + // Non-Image sources must throw, not AV via Unwrap. var ctx = createContext(); var otherCanvas = new _native.Canvas(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage({}, 0, 0);}).to.throw(); diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index 171267f918..e967bf87db 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -662,9 +662,8 @@ describe("Canvas2D", function () { }); it("rejects a non-Image argument to drawImage", function () { - // drawImage used ObjectWrap::Unwrap on whatever object it was handed, so a Canvas, - // Path2D, or plain {} was an access violation rather than a TypeError. - const ctx = createContext(); + // Non-Image sources must throw, not AV via Unwrap. + const ctx = createContext(); const otherCanvas = new _native.Canvas(); expect(function () { ctx.drawImage({}, 0, 0); }).to.throw(); expect(function () { ctx.drawImage(new Path2D(), 0, 0); }).to.throw(); diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 384e31249d..5a092bf7fa 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -204,13 +204,13 @@ namespace Babylon::Polyfills::Internal CanvasGradient* gradient = CanvasGradient::TryUnwrap(info.Env(), std::get(m_state.fillStyle)->Value()); if (gradient == nullptr) { - throw Napi::Error::New(info.Env(), "Fillstyle is not a color string or a gradient."); + throw Napi::TypeError::New(info.Env(), "fillStyle must be a string or CanvasGradient."); } nvgFillPaint(*m_nvg, gradient->Paint()); } else { - throw Napi::Error::New(info.Env(), "Fillstyle is not a color string or a gradient."); + throw Napi::TypeError::New(info.Env(), "fillStyle must be a string or CanvasGradient."); } } @@ -230,13 +230,13 @@ namespace Babylon::Polyfills::Internal CanvasGradient* gradient = CanvasGradient::TryUnwrap(info.Env(), std::get(m_state.strokeStyle)->Value()); if (gradient == nullptr) { - throw Napi::Error::New(info.Env(), "Strokestyle is not a color string or a gradient."); + throw Napi::TypeError::New(info.Env(), "strokeStyle must be a string or CanvasGradient."); } nvgStrokePaint(*m_nvg, gradient->Paint()); } else { - throw Napi::Error::New(info.Env(), "Strokestyle is not a color string or a gradient."); + throw Napi::TypeError::New(info.Env(), "strokeStyle must be a string or CanvasGradient."); } } @@ -354,10 +354,8 @@ namespace Babylon::Polyfills::Internal SetFilterStack(); // TODO: handle fillRule: nonzero, evenodd - // fill() takes either a fillRule string or a Path2D (optionally followed by one). - // Anything else is rejected rather than unwrapped: Unwrap does no type checking, so - // an arbitrary object would be reinterpreted as a NativeCanvasPath2D. - const NativeCanvasPath2D* path = nullptr; + // fill(path?, fillRule?) — reject non-Path2D / non-string first arg. + const NativeCanvasPath2D* path = nullptr; if (info.Length() >= 1 && !info[0].IsUndefined()) { path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); @@ -671,9 +669,8 @@ namespace Babylon::Polyfills::Internal void Context::Stroke(const Napi::CallbackInfo& info) { - // stroke() takes nothing or a Path2D. The argument used to be unwrapped with no type - // check at all, so ctx.stroke("x") cast a string straight into Unwrap. - const NativeCanvasPath2D* path = nullptr; + // stroke(path?) — reject non-Path2D first arg. + const NativeCanvasPath2D* path = nullptr; if (info.Length() >= 1 && !info[0].IsUndefined()) { path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); diff --git a/Polyfills/Canvas/Source/Gradient.cpp b/Polyfills/Canvas/Source/Gradient.cpp index 1c0d41499c..2e30c09e0c 100644 --- a/Polyfills/Canvas/Source/Gradient.cpp +++ b/Polyfills/Canvas/Source/Gradient.cpp @@ -164,8 +164,8 @@ namespace Babylon::Polyfills::Internal r1 = info[5].As().FloatValue(); } - // Registered last: a constructor that throws never reaches the destructor. - NativeInstanceRegistry::Add(info, this); + // Register after successful construction only. + NativeInstanceRegistry::Add(info, this); } CanvasGradient::~CanvasGradient() diff --git a/Polyfills/Canvas/Source/Image.cpp b/Polyfills/Canvas/Source/Image.cpp index 22b3472531..549fe1cac0 100644 --- a/Polyfills/Canvas/Source/Image.cpp +++ b/Polyfills/Canvas/Source/Image.cpp @@ -54,8 +54,8 @@ namespace Babylon::Polyfills::Internal , m_runtimeScheduler{JsRuntime::GetFromJavaScript(info.Env())} , m_cancellationSource{std::make_shared()} { - // Registered last: a constructor that throws never reaches the destructor. - NativeInstanceRegistry::Add(info, this); + // Register after successful construction only. + NativeInstanceRegistry::Add(info, this); } NativeCanvasImage::~NativeCanvasImage() diff --git a/Polyfills/Canvas/Source/Image.h b/Polyfills/Canvas/Source/Image.h index 11ea362a78..8c92f39f50 100644 --- a/Polyfills/Canvas/Source/Image.h +++ b/Polyfills/Canvas/Source/Image.h @@ -19,8 +19,7 @@ namespace Babylon::Polyfills::Internal public: static void Initialize(Napi::Env env); - // Safe brand check — never use ObjectWrap::Unwrap on an unchecked JS value. - static NativeCanvasImage* TryUnwrap(Napi::Env env, const Napi::Value& value); + // static NativeCanvasImage* TryUnwrap(Napi::Env env, const Napi::Value& value); explicit NativeCanvasImage(const Napi::CallbackInfo& info); virtual ~NativeCanvasImage(); diff --git a/Polyfills/Canvas/Source/NativeInstanceRegistry.h b/Polyfills/Canvas/Source/NativeInstanceRegistry.h index d0bfd179fa..21aa5325a6 100644 --- a/Polyfills/Canvas/Source/NativeInstanceRegistry.h +++ b/Polyfills/Canvas/Source/NativeInstanceRegistry.h @@ -6,35 +6,22 @@ namespace Babylon::Polyfills::Internal { - // Answers "is this JS object wrapping a T", and hands back the instance. - // - // `instanceof` cannot answer it: a prototype is assignable, so - // Object.setPrototypeOf(gradient, Path2D.prototype) makes a gradient pass a Path2D test, - // and Object.create(Path2D.prototype) passes with no native object behind it at all. - // - // Neither can ObjectWrap::Unwrap. Both JsRuntimeHost Node-API ports break the contract - // that it fails for an object that was never wrapped: the V8 port dereferences internal - // field 0 unconditionally, which access-violates, and the QuickJS port falls back to - // walking the prototype chain, which returns some other object's pointer. - // - // So each instance brands its own JS object with an External holding its address, and a - // candidate is accepted only if that address is still registered here. An External is - // opaque to script, and the address is compared, never dereferenced, before it is - // accepted, so both spoofs above are rejected instead of being misread as a T. - // - // Script can still copy a brand off a real instance onto another object. That is not a - // memory-safety hole: the result is the live instance the brand came from, which script - // had to already hold. A brand left over from a collected instance is rejected, since the - // destructor unregisters the address. + // Brands a native ObjectWrap instance so TryUnwrap can reject forgeable instanceof / + // unchecked ObjectWrap::Unwrap (V8 AV; QuickJS wrong pointer). Temporary until + // JsRuntimeHost type tags land on all ports; then swap to CheckTypeTag. template class NativeInstanceRegistry { public: - // Call at the very end of the constructor: one that throws never reaches the - // destructor, which would leave a dangling address registered. + // Register at end of ctor only — a throwing ctor never reaches the dtor. static void Add(const Napi::CallbackInfo& info, T* instance) { - info.This().As().Set(BRAND_NAME, Napi::External::New(info.Env(), instance)); + // Non-enumerable brand so it does not show up in Object.keys / for..in. + info.This().As().DefineProperty( + Napi::PropertyDescriptor::Value( + BRAND_NAME, + Napi::External::New(info.Env(), instance), + napi_default)); const std::scoped_lock lock{Mutex()}; Instances().insert(instance); @@ -46,7 +33,6 @@ namespace Babylon::Polyfills::Internal Instances().erase(instance); } - // Returns the wrapped instance, or nullptr when `value` is not one. static T* TryUnwrap(Napi::Env env, const Napi::Value& value) { if (!value.IsObject()) @@ -54,7 +40,6 @@ namespace Babylon::Polyfills::Internal return nullptr; } - // `value` is arbitrary, so the read can run a script accessor that throws. Napi::Value brand{env.Undefined()}; try { @@ -81,12 +66,9 @@ namespace Babylon::Polyfills::Internal } private: - // Shared by every T: a brand read as the wrong type is still rejected, because each T - // registers into its own set. static constexpr const char* BRAND_NAME{"__nativeInstance"}; - // Never destroyed, so that an instance finalized during static destruction cannot - // erase itself from an already-destroyed set. + // Heap-allocated so a late finalizer during static teardown cannot touch a dead set. static std::mutex& Mutex() { static auto* mutex{new std::mutex{}}; @@ -99,4 +81,4 @@ namespace Babylon::Polyfills::Internal return *instances; } }; -} +} \ No newline at end of file diff --git a/Polyfills/Canvas/Source/Path2D.cpp b/Polyfills/Canvas/Source/Path2D.cpp index 073b7662d9..82477610e1 100644 --- a/Polyfills/Canvas/Source/Path2D.cpp +++ b/Polyfills/Canvas/Source/Path2D.cpp @@ -73,11 +73,8 @@ namespace Babylon::Polyfills::Internal : Napi::ObjectWrap{info} , m_commands{std::deque()} { - // new Path2D(path) copies another Path2D; new Path2D(d) parses SVG path data. The - // argument used to be routed by IsObject(), so any object at all was fed to Unwrap and - // reinterpreted as a NativeCanvasPath2D. Per the (Path2D or DOMString) union, a value - // that is not a Path2D is converted to a string instead. - const NativeCanvasPath2D* path = info.Length() >= 1 + // Path2D | DOMString union: only a real Path2D is copied; everything else stringifies. + const NativeCanvasPath2D* path = info.Length() >= 1 ? NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]) : nullptr; const std::string d = path == nullptr && info.Length() >= 1 && !info[0].IsUndefined() @@ -135,8 +132,8 @@ namespace Babylon::Polyfills::Internal nsvg__deleteParser(parser); } - // Registered last: a constructor that throws never reaches the destructor. - NativeInstanceRegistry::Add(info, this); + // Register after successful construction only. + NativeInstanceRegistry::Add(info, this); } NativeCanvasPath2D::~NativeCanvasPath2D() @@ -171,10 +168,7 @@ namespace Babylon::Polyfills::Internal void NativeCanvasPath2D::AddPath(const Napi::CallbackInfo& info) { - // addPath requires a Path2D. There was no type check and no arity check, so - // addPath() read a missing argument and addPath("x") cast a string, both of - // which reached Unwrap. - if (info.Length() < 1) + if (info.Length() < 1) { throw Napi::TypeError::New(info.Env(), "Path2D.addPath: requires at least 1 argument (path)."); } From 80ece475ab2f4889772a63b908f438d691c83d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 1 Sep 2026 18:08:26 -0700 Subject: [PATCH 07/12] Restore NativeCanvasImage::TryUnwrap declaration and fix ctor indent. Header had the declaration commented out while Image.cpp/Context.cpp still define and call it, which broke every Canvas build after the branding work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601 --- Polyfills/Canvas/Source/Image.cpp | 2 +- Polyfills/Canvas/Source/Image.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Polyfills/Canvas/Source/Image.cpp b/Polyfills/Canvas/Source/Image.cpp index 549fe1cac0..af8489bacd 100644 --- a/Polyfills/Canvas/Source/Image.cpp +++ b/Polyfills/Canvas/Source/Image.cpp @@ -55,7 +55,7 @@ namespace Babylon::Polyfills::Internal , m_cancellationSource{std::make_shared()} { // Register after successful construction only. - NativeInstanceRegistry::Add(info, this); + NativeInstanceRegistry::Add(info, this); } NativeCanvasImage::~NativeCanvasImage() diff --git a/Polyfills/Canvas/Source/Image.h b/Polyfills/Canvas/Source/Image.h index 8c92f39f50..5c3b28a604 100644 --- a/Polyfills/Canvas/Source/Image.h +++ b/Polyfills/Canvas/Source/Image.h @@ -19,7 +19,7 @@ namespace Babylon::Polyfills::Internal public: static void Initialize(Napi::Env env); - // static NativeCanvasImage* TryUnwrap(Napi::Env env, const Napi::Value& value); + static NativeCanvasImage* TryUnwrap(Napi::Env env, const Napi::Value& value); explicit NativeCanvasImage(const Napi::CallbackInfo& info); virtual ~NativeCanvasImage(); From acf34a2e7a3c2a77ed6e6674048cdd712ad7c18b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Tue, 1 Sep 2026 18:08:49 -0700 Subject: [PATCH 08/12] Fix over-indented NativeInstanceRegistry::Add in Gradient/Path2D ctors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601 --- Polyfills/Canvas/Source/Gradient.cpp | 2 +- Polyfills/Canvas/Source/Path2D.cpp | 798 +++++++++++++-------------- 2 files changed, 400 insertions(+), 400 deletions(-) diff --git a/Polyfills/Canvas/Source/Gradient.cpp b/Polyfills/Canvas/Source/Gradient.cpp index 2e30c09e0c..1f7d533561 100644 --- a/Polyfills/Canvas/Source/Gradient.cpp +++ b/Polyfills/Canvas/Source/Gradient.cpp @@ -165,7 +165,7 @@ namespace Babylon::Polyfills::Internal } // Register after successful construction only. - NativeInstanceRegistry::Add(info, this); + NativeInstanceRegistry::Add(info, this); } CanvasGradient::~CanvasGradient() diff --git a/Polyfills/Canvas/Source/Path2D.cpp b/Polyfills/Canvas/Source/Path2D.cpp index 82477610e1..0c524b853c 100644 --- a/Polyfills/Canvas/Source/Path2D.cpp +++ b/Polyfills/Canvas/Source/Path2D.cpp @@ -1,399 +1,399 @@ -#include -#include -#include -#include "Canvas.h" -#include "NativeInstanceRegistry.h" -#include "Path2D.h" -#include - -#ifdef __GNUC__ -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wpedantic" -#endif - -#define NANOSVG_IMPLEMENTATION // Expands implementation -#include "nanosvg.h" - -#ifdef __GNUC__ -#pragma GCC diagnostic pop -#endif - -namespace Babylon::Polyfills::Internal -{ - static constexpr auto JS_PATH2D_CONSTRUCTOR_NAME = "Path2D"; - - void NativeCanvasPath2D::Initialize(Napi::Env env) - { - Napi::HandleScope scope{env}; - - Napi::Function func = DefineClass( - env, - JS_PATH2D_CONSTRUCTOR_NAME, - { - InstanceMethod("addPath", &NativeCanvasPath2D::AddPath), - InstanceMethod("closePath", &NativeCanvasPath2D::ClosePath), - InstanceMethod("moveTo", &NativeCanvasPath2D::MoveTo), - InstanceMethod("lineTo", &NativeCanvasPath2D::LineTo), - InstanceMethod("bezierCurveTo", &NativeCanvasPath2D::BezierCurveTo), - InstanceMethod("quadraticCurveTo", &NativeCanvasPath2D::QuadraticCurveTo), - InstanceMethod("arc", &NativeCanvasPath2D::Arc), - InstanceMethod("arcTo", &NativeCanvasPath2D::ArcTo), - InstanceMethod("ellipse", &NativeCanvasPath2D::Ellipse), - InstanceMethod("rect", &NativeCanvasPath2D::Rect), - InstanceMethod("roundRect", &NativeCanvasPath2D::RoundRect), - }); - - JsRuntime::NativeObject::GetFromJavaScript(env).Set(JS_PATH2D_CONSTRUCTOR_NAME, func); - - // Browsers expose Path2D as a global constructor, and Babylon.js relies on that: the generic - // engines call `new Path2D(d)` from AbstractEngine.createCanvasPath2D. Only NativeEngine - // overrides that method to use `_native.Path2D`, so without the global any portable browser - // code doing `new Path2D(...)` fails with "Path2D is not defined". - auto global = env.Global(); - - // Claim the global unconditionally. A Path2D from anywhere else cannot be drawn by this - // Context -- Context::Fill unwraps whatever object it is handed as a NativeCanvasPath2D -- - // so deferring to a foreign constructor would only guarantee that `ctx.fill(new Path2D(d))` - // unwraps an object that was never wrapped. Canvas::Initialize calls this while the - // polyfill is still installing, so anything already there came from outside it. - global.Set(JS_PATH2D_CONSTRUCTOR_NAME, func); - } - - bool NativeCanvasPath2D::IsInstance(Napi::Env env, const Napi::Value& value) - { - return TryUnwrap(env, value) != nullptr; - } - - NativeCanvasPath2D* NativeCanvasPath2D::TryUnwrap(Napi::Env env, const Napi::Value& value) - { - return NativeInstanceRegistry::TryUnwrap(env, value); - } - - NativeCanvasPath2D::NativeCanvasPath2D(const Napi::CallbackInfo& info) - : Napi::ObjectWrap{info} - , m_commands{std::deque()} - { - // Path2D | DOMString union: only a real Path2D is copied; everything else stringifies. - const NativeCanvasPath2D* path = info.Length() >= 1 - ? NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]) - : nullptr; - const std::string d = path == nullptr && info.Length() >= 1 && !info[0].IsUndefined() - ? info[0].ToString().Utf8Value() - : ""; - - if (path != nullptr) - { - for (const auto& command : *path) - { - m_commands.push_back(command); - } - } - - if (!d.empty()) - { - NSVGparser* parser = nsvg__createParser(); - const char* path[] = {"d", d.c_str(), NULL}; - const char** attr = {path}; - - assert(strcmp(attr[0], "d") == 0); - assert(!attr[2]); // nsvg__parsePath terminates attr parsing on falsy - - nsvg__parsePath(parser, attr); - - for (NSVGshape *shape = parser->image->shapes; shape != NULL; shape = shape->next) { - for (NSVGpath *path = shape->paths; path != NULL; path = path->next) { - for (int i = 0; i < path->npts-1; i += 3) { - float* p = &path->pts[i*2]; - - auto x0 = p[0]; // start x, same as end x of previous - auto y0 = p[1]; // start y, same as end y of previous - auto cpx1 = p[2]; - auto cpy1 = p[3]; - auto cpx2 = p[4]; - auto cpy2 = p[5]; - auto x1 = p[6]; // end x - auto y1 = p[7]; // end y - - // Only need to move on new shape - if (i == 0) - { - Path2DCommandArgs moveArgs = {}; - moveArgs.moveTo = {x0, y0}; - AppendCommand(P2D_MOVETO, moveArgs); - } - - Path2DCommandArgs args = {}; - args.bezierTo = {cpx1, cpy1, cpx2, cpy2, x1, y1}; - AppendCommand(P2D_BEZIERTO, args); - } - } - } - - nsvg__deleteParser(parser); - } - - // Register after successful construction only. - NativeInstanceRegistry::Add(info, this); - } - - NativeCanvasPath2D::~NativeCanvasPath2D() - { - NativeInstanceRegistry::Remove(this); - } - - typename std::deque::iterator NativeCanvasPath2D::begin() - { - return m_commands.begin(); - } - - typename std::deque::iterator NativeCanvasPath2D::end() - { - return m_commands.end(); - } - - typename std::deque::const_iterator NativeCanvasPath2D::begin() const - { - return m_commands.begin(); - } - - typename std::deque::const_iterator NativeCanvasPath2D::end() const - { - return m_commands.end(); - } - - void NativeCanvasPath2D::AppendCommand(Path2DCommandTypes type, Path2DCommandArgs args) - { - m_commands.push_back({type, args}); - } - - void NativeCanvasPath2D::AddPath(const Napi::CallbackInfo& info) - { - if (info.Length() < 1) - { - throw Napi::TypeError::New(info.Env(), "Path2D.addPath: requires at least 1 argument (path)."); - } - - NativeCanvasPath2D* const path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); - if (path == nullptr) - { - throw Napi::TypeError::New(info.Env(), "Path2D.addPath: the first argument is not a Path2D."); - } - - // optional transform arg - bool xformInvReady{false}; - float xformInv[6]; - if (info.Length() == 2) - { - Napi::Object transform = info[1].As(); - auto a = transform.Get("a").As().FloatValue(); - auto b = transform.Get("b").As().FloatValue(); - auto c = transform.Get("c").As().FloatValue(); - auto d = transform.Get("d").As().FloatValue(); - auto e = transform.Get("e").As().FloatValue(); - auto f = transform.Get("f").As().FloatValue(); - - float xform[6] = {a, b, c, d, e, f}; - nsvg__xformInverse(xformInv, xform); - xformInvReady = true; - - Path2DCommandArgs args = {}; - args.transform = { xform[0], xform[1], xform[2], xform[3], xform[4], xform[5] }; - AppendCommand(P2D_TRANSFORM, args); - } - - for (const auto& command : *path) - { - m_commands.push_back(command); - } - - // invert transform after all commands played - if (info.Length() == 2) - { - assert(xformInvReady); - Path2DCommandArgs argsInv = {}; - argsInv.transform = {xformInv[0], xformInv[1], xformInv[2], xformInv[3], xformInv[4], xformInv[5]}; - AppendCommand(P2D_TRANSFORM, argsInv); - } - } - - void NativeCanvasPath2D::ClosePath(const Napi::CallbackInfo& info) - { - AppendCommand(P2D_CLOSE, {}); - } - - void NativeCanvasPath2D::MoveTo(const Napi::CallbackInfo& info) - { - const auto x = static_cast(info[0].As().DoubleValue()); - const auto y = static_cast(info[1].As().DoubleValue()); - - Path2DCommandArgs args = {}; - args.moveTo = {x, y}; - AppendCommand(P2D_MOVETO, args); - } - - void NativeCanvasPath2D::LineTo(const Napi::CallbackInfo& info) - { - const auto x = static_cast(info[0].As().DoubleValue()); - const auto y = static_cast(info[1].As().DoubleValue()); - - Path2DCommandArgs args = {}; - args.lineTo = {x, y}; - AppendCommand(P2D_LINETO, args); - } - - void NativeCanvasPath2D::BezierCurveTo(const Napi::CallbackInfo& info) - { - const auto cp1x = static_cast(info[0].As().DoubleValue()); - const auto cp1y = static_cast(info[1].As().DoubleValue()); - const auto cp2x = static_cast(info[2].As().DoubleValue()); - const auto cp2y = static_cast(info[3].As().DoubleValue()); - const auto x = static_cast(info[4].As().DoubleValue()); - const auto y = static_cast(info[5].As().DoubleValue()); - - Path2DCommandArgs args = {}; - args.bezierTo = {cp1x, cp1y, cp2x, cp2y, x, y}; - AppendCommand(P2D_BEZIERTO, args); - } - - void NativeCanvasPath2D::QuadraticCurveTo(const Napi::CallbackInfo& info) - { - const auto cpx = static_cast(info[0].As().DoubleValue()); - const auto cpy = static_cast(info[1].As().DoubleValue()); - const auto x = static_cast(info[2].As().DoubleValue()); - const auto y = static_cast(info[3].As().DoubleValue()); - - Path2DCommandArgs args = {}; - args.quadTo = {cpx, cpy, x, y}; - AppendCommand(P2D_QUADTO, args); - } - - void NativeCanvasPath2D::Arc(const Napi::CallbackInfo& info) - { - const auto x = static_cast(info[0].As().DoubleValue()); - const auto y = static_cast(info[1].As().DoubleValue()); - const auto radius = static_cast(info[2].As().DoubleValue()); - const auto startAngle = static_cast(info[3].As().DoubleValue()); - const auto endAngle = static_cast(info[4].As().DoubleValue()); - const auto counterclockwise = info.Length() == 6 ? info[5].As() : false; - - Path2DCommandArgs args = {}; - args.arc = {x, y, radius, startAngle, endAngle, counterclockwise}; - AppendCommand(P2D_ARC, args); - } - - void NativeCanvasPath2D::ArcTo(const Napi::CallbackInfo& info) - { - const auto x1 = static_cast(info[0].As().DoubleValue()); - const auto y1 = static_cast(info[1].As().DoubleValue()); - const auto x2 = static_cast(info[2].As().DoubleValue()); - const auto y2 = static_cast(info[3].As().DoubleValue()); - const auto radius = static_cast(info[4].As().DoubleValue()); - - Path2DCommandArgs args = {}; - args.arcTo = {x1, y1, x2, y2, radius}; - AppendCommand(P2D_ARCTO, args); - } - - void NativeCanvasPath2D::Ellipse(const Napi::CallbackInfo& info) - { - const auto x = static_cast(info[0].As().DoubleValue()); - const auto y = static_cast(info[1].As().DoubleValue()); - const auto radiusX = static_cast(info[2].As().DoubleValue()); - const auto radiusY = static_cast(info[3].As().DoubleValue()); - const auto rotation = static_cast(info[4].As().DoubleValue()); - const auto startAngle = static_cast(info[5].As().DoubleValue()); - const auto endAngle = static_cast(info[6].As().DoubleValue()); - const auto counterclockwise = info.Length() == 8 ? info[7].As() : false; - - Path2DCommandArgs args = {}; - args.ellipse = {x, y, radiusX, radiusY, rotation, startAngle, endAngle, counterclockwise}; - AppendCommand(P2D_ELLIPSE, args); - } - - void NativeCanvasPath2D::Rect(const Napi::CallbackInfo& info) - { - const auto x = static_cast(info[0].As().DoubleValue()); - const auto y = static_cast(info[1].As().DoubleValue()); - const auto width = static_cast(info[2].As().DoubleValue()); - const auto height = static_cast(info[3].As().DoubleValue()); - - Path2DCommandArgs args = {}; - args.rect = {x, y, width, height}; - AppendCommand(P2D_RECT, args); - } - - void NativeCanvasPath2D::RoundRect(const Napi::CallbackInfo& info) - { - const auto x = static_cast(info[0].As().DoubleValue()); - const auto y = static_cast(info[1].As().DoubleValue()); - const auto width = static_cast(info[2].As().DoubleValue()); - const auto height = static_cast(info[3].As().DoubleValue()); - const auto radii = info[4]; - - if (radii.IsNumber()) - { - const auto radius = radii.As().FloatValue(); - Path2DCommandArgs args = {}; - args.roundRect = {x, y, width, height, radius}; - AppendCommand(P2D_ROUNDRECT, args); - } - else if (radii.IsArray()) - { - const auto radiiArray = radii.As(); - const auto radiiArrayLength = radiiArray.Length(); - if (radiiArrayLength == 1) - { - const auto radius = radiiArray[0u].As().FloatValue(); - Path2DCommandArgs args = {}; - args.roundRect = {x, y, width, height, radius}; - AppendCommand(P2D_ROUNDRECT, args); - } - else if (radiiArrayLength == 2) - { - const auto topLeftBottomRight = radiiArray[0u].As().FloatValue(); - const auto topRightBottomLeft = radiiArray[1u].As().FloatValue(); - Path2DCommandArgs args = {}; - args.roundRectVarying = {x, y, width, height, topLeftBottomRight, topRightBottomLeft, topLeftBottomRight, topRightBottomLeft}; - AppendCommand(P2D_ROUNDRECTVARYING, args); - } - else if (radiiArrayLength == 3) - { - const auto topLeft = radiiArray[0u].As().FloatValue(); - const auto topRightBottomLeft = radiiArray[1u].As().FloatValue(); - const auto bottomRight = radiiArray[2u].As().FloatValue(); - Path2DCommandArgs args = {}; - args.roundRectVarying = {x, y, width, height, topLeft, topRightBottomLeft, bottomRight, topRightBottomLeft}; - AppendCommand(P2D_ROUNDRECTVARYING, args); - } - else if (radiiArrayLength == 4) - { - const auto topLeft = radiiArray[0u].As().FloatValue(); - const auto topRight = radiiArray[1u].As().FloatValue(); - const auto bottomRight = radiiArray[2u].As().FloatValue(); - const auto bottomLeft = radiiArray[3u].As().FloatValue(); - Path2DCommandArgs args = {}; - args.roundRectVarying = {x, y, width, height, topLeft, topRight, bottomRight, bottomLeft}; - AppendCommand(P2D_ROUNDRECTVARYING, args); - } - else - { - throw Napi::Error::New(info.Env(), "Invalid number of parameters for radii"); - } - } - // DOMPoint - // TODO: move duplicate Path2D & Context args parsing into a utils.cpp - else if (radii.IsObject()) - { - const auto dompoint = radii.As(); - const auto radiusX = dompoint.Get("x").As().FloatValue(); - const auto radiusY = dompoint.Get("y").As().FloatValue(); - Path2DCommandArgs args = {}; - args.roundRectElliptic = {x, y, width, height, radiusX, radiusY, radiusX, radiusY, radiusX, radiusY, radiusX, radiusY}; - AppendCommand(P2D_ROUNDRECTELLIPTIC, args); - } - else - { - throw Napi::Error::New(info.Env(), "Invalid radii parameter"); - } - } -} +#include +#include +#include +#include "Canvas.h" +#include "NativeInstanceRegistry.h" +#include "Path2D.h" +#include + +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wpedantic" +#endif + +#define NANOSVG_IMPLEMENTATION // Expands implementation +#include "nanosvg.h" + +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif + +namespace Babylon::Polyfills::Internal +{ + static constexpr auto JS_PATH2D_CONSTRUCTOR_NAME = "Path2D"; + + void NativeCanvasPath2D::Initialize(Napi::Env env) + { + Napi::HandleScope scope{env}; + + Napi::Function func = DefineClass( + env, + JS_PATH2D_CONSTRUCTOR_NAME, + { + InstanceMethod("addPath", &NativeCanvasPath2D::AddPath), + InstanceMethod("closePath", &NativeCanvasPath2D::ClosePath), + InstanceMethod("moveTo", &NativeCanvasPath2D::MoveTo), + InstanceMethod("lineTo", &NativeCanvasPath2D::LineTo), + InstanceMethod("bezierCurveTo", &NativeCanvasPath2D::BezierCurveTo), + InstanceMethod("quadraticCurveTo", &NativeCanvasPath2D::QuadraticCurveTo), + InstanceMethod("arc", &NativeCanvasPath2D::Arc), + InstanceMethod("arcTo", &NativeCanvasPath2D::ArcTo), + InstanceMethod("ellipse", &NativeCanvasPath2D::Ellipse), + InstanceMethod("rect", &NativeCanvasPath2D::Rect), + InstanceMethod("roundRect", &NativeCanvasPath2D::RoundRect), + }); + + JsRuntime::NativeObject::GetFromJavaScript(env).Set(JS_PATH2D_CONSTRUCTOR_NAME, func); + + // Browsers expose Path2D as a global constructor, and Babylon.js relies on that: the generic + // engines call `new Path2D(d)` from AbstractEngine.createCanvasPath2D. Only NativeEngine + // overrides that method to use `_native.Path2D`, so without the global any portable browser + // code doing `new Path2D(...)` fails with "Path2D is not defined". + auto global = env.Global(); + + // Claim the global unconditionally. A Path2D from anywhere else cannot be drawn by this + // Context -- Context::Fill unwraps whatever object it is handed as a NativeCanvasPath2D -- + // so deferring to a foreign constructor would only guarantee that `ctx.fill(new Path2D(d))` + // unwraps an object that was never wrapped. Canvas::Initialize calls this while the + // polyfill is still installing, so anything already there came from outside it. + global.Set(JS_PATH2D_CONSTRUCTOR_NAME, func); + } + + bool NativeCanvasPath2D::IsInstance(Napi::Env env, const Napi::Value& value) + { + return TryUnwrap(env, value) != nullptr; + } + + NativeCanvasPath2D* NativeCanvasPath2D::TryUnwrap(Napi::Env env, const Napi::Value& value) + { + return NativeInstanceRegistry::TryUnwrap(env, value); + } + + NativeCanvasPath2D::NativeCanvasPath2D(const Napi::CallbackInfo& info) + : Napi::ObjectWrap{info} + , m_commands{std::deque()} + { + // Path2D | DOMString union: only a real Path2D is copied; everything else stringifies. + const NativeCanvasPath2D* path = info.Length() >= 1 + ? NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]) + : nullptr; + const std::string d = path == nullptr && info.Length() >= 1 && !info[0].IsUndefined() + ? info[0].ToString().Utf8Value() + : ""; + + if (path != nullptr) + { + for (const auto& command : *path) + { + m_commands.push_back(command); + } + } + + if (!d.empty()) + { + NSVGparser* parser = nsvg__createParser(); + const char* path[] = {"d", d.c_str(), NULL}; + const char** attr = {path}; + + assert(strcmp(attr[0], "d") == 0); + assert(!attr[2]); // nsvg__parsePath terminates attr parsing on falsy + + nsvg__parsePath(parser, attr); + + for (NSVGshape *shape = parser->image->shapes; shape != NULL; shape = shape->next) { + for (NSVGpath *path = shape->paths; path != NULL; path = path->next) { + for (int i = 0; i < path->npts-1; i += 3) { + float* p = &path->pts[i*2]; + + auto x0 = p[0]; // start x, same as end x of previous + auto y0 = p[1]; // start y, same as end y of previous + auto cpx1 = p[2]; + auto cpy1 = p[3]; + auto cpx2 = p[4]; + auto cpy2 = p[5]; + auto x1 = p[6]; // end x + auto y1 = p[7]; // end y + + // Only need to move on new shape + if (i == 0) + { + Path2DCommandArgs moveArgs = {}; + moveArgs.moveTo = {x0, y0}; + AppendCommand(P2D_MOVETO, moveArgs); + } + + Path2DCommandArgs args = {}; + args.bezierTo = {cpx1, cpy1, cpx2, cpy2, x1, y1}; + AppendCommand(P2D_BEZIERTO, args); + } + } + } + + nsvg__deleteParser(parser); + } + + // Register after successful construction only. + NativeInstanceRegistry::Add(info, this); + } + + NativeCanvasPath2D::~NativeCanvasPath2D() + { + NativeInstanceRegistry::Remove(this); + } + + typename std::deque::iterator NativeCanvasPath2D::begin() + { + return m_commands.begin(); + } + + typename std::deque::iterator NativeCanvasPath2D::end() + { + return m_commands.end(); + } + + typename std::deque::const_iterator NativeCanvasPath2D::begin() const + { + return m_commands.begin(); + } + + typename std::deque::const_iterator NativeCanvasPath2D::end() const + { + return m_commands.end(); + } + + void NativeCanvasPath2D::AppendCommand(Path2DCommandTypes type, Path2DCommandArgs args) + { + m_commands.push_back({type, args}); + } + + void NativeCanvasPath2D::AddPath(const Napi::CallbackInfo& info) + { + if (info.Length() < 1) + { + throw Napi::TypeError::New(info.Env(), "Path2D.addPath: requires at least 1 argument (path)."); + } + + NativeCanvasPath2D* const path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); + if (path == nullptr) + { + throw Napi::TypeError::New(info.Env(), "Path2D.addPath: the first argument is not a Path2D."); + } + + // optional transform arg + bool xformInvReady{false}; + float xformInv[6]; + if (info.Length() == 2) + { + Napi::Object transform = info[1].As(); + auto a = transform.Get("a").As().FloatValue(); + auto b = transform.Get("b").As().FloatValue(); + auto c = transform.Get("c").As().FloatValue(); + auto d = transform.Get("d").As().FloatValue(); + auto e = transform.Get("e").As().FloatValue(); + auto f = transform.Get("f").As().FloatValue(); + + float xform[6] = {a, b, c, d, e, f}; + nsvg__xformInverse(xformInv, xform); + xformInvReady = true; + + Path2DCommandArgs args = {}; + args.transform = { xform[0], xform[1], xform[2], xform[3], xform[4], xform[5] }; + AppendCommand(P2D_TRANSFORM, args); + } + + for (const auto& command : *path) + { + m_commands.push_back(command); + } + + // invert transform after all commands played + if (info.Length() == 2) + { + assert(xformInvReady); + Path2DCommandArgs argsInv = {}; + argsInv.transform = {xformInv[0], xformInv[1], xformInv[2], xformInv[3], xformInv[4], xformInv[5]}; + AppendCommand(P2D_TRANSFORM, argsInv); + } + } + + void NativeCanvasPath2D::ClosePath(const Napi::CallbackInfo& info) + { + AppendCommand(P2D_CLOSE, {}); + } + + void NativeCanvasPath2D::MoveTo(const Napi::CallbackInfo& info) + { + const auto x = static_cast(info[0].As().DoubleValue()); + const auto y = static_cast(info[1].As().DoubleValue()); + + Path2DCommandArgs args = {}; + args.moveTo = {x, y}; + AppendCommand(P2D_MOVETO, args); + } + + void NativeCanvasPath2D::LineTo(const Napi::CallbackInfo& info) + { + const auto x = static_cast(info[0].As().DoubleValue()); + const auto y = static_cast(info[1].As().DoubleValue()); + + Path2DCommandArgs args = {}; + args.lineTo = {x, y}; + AppendCommand(P2D_LINETO, args); + } + + void NativeCanvasPath2D::BezierCurveTo(const Napi::CallbackInfo& info) + { + const auto cp1x = static_cast(info[0].As().DoubleValue()); + const auto cp1y = static_cast(info[1].As().DoubleValue()); + const auto cp2x = static_cast(info[2].As().DoubleValue()); + const auto cp2y = static_cast(info[3].As().DoubleValue()); + const auto x = static_cast(info[4].As().DoubleValue()); + const auto y = static_cast(info[5].As().DoubleValue()); + + Path2DCommandArgs args = {}; + args.bezierTo = {cp1x, cp1y, cp2x, cp2y, x, y}; + AppendCommand(P2D_BEZIERTO, args); + } + + void NativeCanvasPath2D::QuadraticCurveTo(const Napi::CallbackInfo& info) + { + const auto cpx = static_cast(info[0].As().DoubleValue()); + const auto cpy = static_cast(info[1].As().DoubleValue()); + const auto x = static_cast(info[2].As().DoubleValue()); + const auto y = static_cast(info[3].As().DoubleValue()); + + Path2DCommandArgs args = {}; + args.quadTo = {cpx, cpy, x, y}; + AppendCommand(P2D_QUADTO, args); + } + + void NativeCanvasPath2D::Arc(const Napi::CallbackInfo& info) + { + const auto x = static_cast(info[0].As().DoubleValue()); + const auto y = static_cast(info[1].As().DoubleValue()); + const auto radius = static_cast(info[2].As().DoubleValue()); + const auto startAngle = static_cast(info[3].As().DoubleValue()); + const auto endAngle = static_cast(info[4].As().DoubleValue()); + const auto counterclockwise = info.Length() == 6 ? info[5].As() : false; + + Path2DCommandArgs args = {}; + args.arc = {x, y, radius, startAngle, endAngle, counterclockwise}; + AppendCommand(P2D_ARC, args); + } + + void NativeCanvasPath2D::ArcTo(const Napi::CallbackInfo& info) + { + const auto x1 = static_cast(info[0].As().DoubleValue()); + const auto y1 = static_cast(info[1].As().DoubleValue()); + const auto x2 = static_cast(info[2].As().DoubleValue()); + const auto y2 = static_cast(info[3].As().DoubleValue()); + const auto radius = static_cast(info[4].As().DoubleValue()); + + Path2DCommandArgs args = {}; + args.arcTo = {x1, y1, x2, y2, radius}; + AppendCommand(P2D_ARCTO, args); + } + + void NativeCanvasPath2D::Ellipse(const Napi::CallbackInfo& info) + { + const auto x = static_cast(info[0].As().DoubleValue()); + const auto y = static_cast(info[1].As().DoubleValue()); + const auto radiusX = static_cast(info[2].As().DoubleValue()); + const auto radiusY = static_cast(info[3].As().DoubleValue()); + const auto rotation = static_cast(info[4].As().DoubleValue()); + const auto startAngle = static_cast(info[5].As().DoubleValue()); + const auto endAngle = static_cast(info[6].As().DoubleValue()); + const auto counterclockwise = info.Length() == 8 ? info[7].As() : false; + + Path2DCommandArgs args = {}; + args.ellipse = {x, y, radiusX, radiusY, rotation, startAngle, endAngle, counterclockwise}; + AppendCommand(P2D_ELLIPSE, args); + } + + void NativeCanvasPath2D::Rect(const Napi::CallbackInfo& info) + { + const auto x = static_cast(info[0].As().DoubleValue()); + const auto y = static_cast(info[1].As().DoubleValue()); + const auto width = static_cast(info[2].As().DoubleValue()); + const auto height = static_cast(info[3].As().DoubleValue()); + + Path2DCommandArgs args = {}; + args.rect = {x, y, width, height}; + AppendCommand(P2D_RECT, args); + } + + void NativeCanvasPath2D::RoundRect(const Napi::CallbackInfo& info) + { + const auto x = static_cast(info[0].As().DoubleValue()); + const auto y = static_cast(info[1].As().DoubleValue()); + const auto width = static_cast(info[2].As().DoubleValue()); + const auto height = static_cast(info[3].As().DoubleValue()); + const auto radii = info[4]; + + if (radii.IsNumber()) + { + const auto radius = radii.As().FloatValue(); + Path2DCommandArgs args = {}; + args.roundRect = {x, y, width, height, radius}; + AppendCommand(P2D_ROUNDRECT, args); + } + else if (radii.IsArray()) + { + const auto radiiArray = radii.As(); + const auto radiiArrayLength = radiiArray.Length(); + if (radiiArrayLength == 1) + { + const auto radius = radiiArray[0u].As().FloatValue(); + Path2DCommandArgs args = {}; + args.roundRect = {x, y, width, height, radius}; + AppendCommand(P2D_ROUNDRECT, args); + } + else if (radiiArrayLength == 2) + { + const auto topLeftBottomRight = radiiArray[0u].As().FloatValue(); + const auto topRightBottomLeft = radiiArray[1u].As().FloatValue(); + Path2DCommandArgs args = {}; + args.roundRectVarying = {x, y, width, height, topLeftBottomRight, topRightBottomLeft, topLeftBottomRight, topRightBottomLeft}; + AppendCommand(P2D_ROUNDRECTVARYING, args); + } + else if (radiiArrayLength == 3) + { + const auto topLeft = radiiArray[0u].As().FloatValue(); + const auto topRightBottomLeft = radiiArray[1u].As().FloatValue(); + const auto bottomRight = radiiArray[2u].As().FloatValue(); + Path2DCommandArgs args = {}; + args.roundRectVarying = {x, y, width, height, topLeft, topRightBottomLeft, bottomRight, topRightBottomLeft}; + AppendCommand(P2D_ROUNDRECTVARYING, args); + } + else if (radiiArrayLength == 4) + { + const auto topLeft = radiiArray[0u].As().FloatValue(); + const auto topRight = radiiArray[1u].As().FloatValue(); + const auto bottomRight = radiiArray[2u].As().FloatValue(); + const auto bottomLeft = radiiArray[3u].As().FloatValue(); + Path2DCommandArgs args = {}; + args.roundRectVarying = {x, y, width, height, topLeft, topRight, bottomRight, bottomLeft}; + AppendCommand(P2D_ROUNDRECTVARYING, args); + } + else + { + throw Napi::Error::New(info.Env(), "Invalid number of parameters for radii"); + } + } + // DOMPoint + // TODO: move duplicate Path2D & Context args parsing into a utils.cpp + else if (radii.IsObject()) + { + const auto dompoint = radii.As(); + const auto radiusX = dompoint.Get("x").As().FloatValue(); + const auto radiusY = dompoint.Get("y").As().FloatValue(); + Path2DCommandArgs args = {}; + args.roundRectElliptic = {x, y, width, height, radiusX, radiusY, radiusX, radiusY, radiusX, radiusY, radiusX, radiusY}; + AppendCommand(P2D_ROUNDRECTELLIPTIC, args); + } + else + { + throw Napi::Error::New(info.Env(), "Invalid radii parameter"); + } + } +} From 41528cc72638c19899f513bad27744b6bfbdad5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Wed, 2 Sep 2026 08:53:44 -0700 Subject: [PATCH 09/12] Avoid Napi::PropertyDescriptor in NativeInstanceRegistry (UWP/JSI). UWP arm64 JSI build fails with C2039: PropertyDescriptor is not a member of Napi. Brand instances via napi_define_properties instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d47cbab2-d751-4cf9-984f-4412dd9ec601 --- .../Canvas/Source/NativeInstanceRegistry.h | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/Polyfills/Canvas/Source/NativeInstanceRegistry.h b/Polyfills/Canvas/Source/NativeInstanceRegistry.h index 21aa5325a6..17a516f48b 100644 --- a/Polyfills/Canvas/Source/NativeInstanceRegistry.h +++ b/Polyfills/Canvas/Source/NativeInstanceRegistry.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -17,15 +18,24 @@ namespace Babylon::Polyfills::Internal static void Add(const Napi::CallbackInfo& info, T* instance) { // Non-enumerable brand so it does not show up in Object.keys / for..in. - info.This().As().DefineProperty( - Napi::PropertyDescriptor::Value( - BRAND_NAME, - Napi::External::New(info.Env(), instance), - napi_default)); + // Use the C napi_define_properties path: Napi::PropertyDescriptor is not + // available on every JsRuntimeHost port (notably UWP/JSI). + Napi::Object self = info.This().As(); + Napi::External brand = Napi::External::New(info.Env(), instance); + napi_property_descriptor desc{}; + desc.utf8name = BRAND_NAME; + desc.value = brand; + desc.attributes = napi_default; // non-enumerable, non-configurable, non-writable + napi_status status = napi_define_properties(info.Env(), self, 1, &desc); + if (status != napi_ok) + { + Napi::Error::New(info.Env(), "NativeInstanceRegistry: failed to brand instance").ThrowAsJavaScriptException(); + return; + } - const std::scoped_lock lock{Mutex()}; - Instances().insert(instance); - } + const std::scoped_lock lock{Mutex()}; + Instances().insert(instance); + } static void Remove(const T* instance) { From 76f51713f9ec63fdcbb5ef007e03d84e19f56cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Mon, 14 Sep 2026 12:47:57 -0700 Subject: [PATCH 10/12] Canvas: accept string-convertible fill rules Honor the WebIDL enum conversion used by fill() for boxed strings and other string-convertible values, then validate the converted value. Cover both fill-rule overload positions and invalid enum values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 54aaa1e1-b4b3-46e7-9de4-5a56add4ac42 --- .../JavaScript/dist/tests.javaScript.all.js | 7 +++++++ .../JavaScript/src/tests.javaScript.all.ts | 7 +++++++ Polyfills/Canvas/Source/Context.cpp | 21 +++++++++++++++---- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index a4c95dce43..aed5e5d609 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29147,9 +29147,16 @@ describe("Canvas2D", function () { (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(undefined);}).to.not.throw(); // fill() also takes a fill rule string. (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill("evenodd");}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(new String("evenodd"));}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () { + ctx.fill({ toString: function () {return "nonzero";} }); + }).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path);}).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path, "nonzero");}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path, new String("evenodd"));}).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.stroke(path);}).to.not.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill("invalid");}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path, "invalid");}).to.throw(); }); it("rejects a non-Path2D argument to Path2D.addPath", function () { diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index e967bf87db..72699ec9dd 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -645,9 +645,16 @@ describe("Canvas2D", function () { expect(function () { ctx.stroke(undefined); }).to.not.throw(); // fill() also takes a fill rule string. expect(function () { ctx.fill("evenodd"); }).to.not.throw(); + expect(function () { ctx.fill(new String("evenodd") as any); }).to.not.throw(); + expect(function () { + ctx.fill({ toString: function () { return "nonzero"; } } as any); + }).to.not.throw(); expect(function () { ctx.fill(path); }).to.not.throw(); expect(function () { ctx.fill(path, "nonzero"); }).to.not.throw(); + expect(function () { ctx.fill(path, new String("evenodd") as any); }).to.not.throw(); expect(function () { ctx.stroke(path); }).to.not.throw(); + expect(function () { ctx.fill("invalid"); }).to.throw(); + expect(function () { ctx.fill(path, "invalid"); }).to.throw(); }); it("rejects a non-Path2D argument to Path2D.addPath", function () { diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 5a092bf7fa..71899a2d42 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -62,6 +62,15 @@ namespace Babylon::Polyfills::Internal value <= static_cast(std::numeric_limits::max()) && value == std::trunc(value); } + + void ValidateFillRule(Napi::Env env, const Napi::Value& value) + { + const auto fillRule = value.ToString().Utf8Value(); + if (fillRule != "nonzero" && fillRule != "evenodd") + { + throw Napi::TypeError::New(env, "Context2D.fill: the fill rule must be \"nonzero\" or \"evenodd\"."); + } + } } void Context::Initialize(Napi::Env env) @@ -354,16 +363,20 @@ namespace Babylon::Polyfills::Internal SetFilterStack(); // TODO: handle fillRule: nonzero, evenodd - // fill(path?, fillRule?) — reject non-Path2D / non-string first arg. - const NativeCanvasPath2D* path = nullptr; + // fill(path?, fillRule?) — distinguish Path2D from the string-converted enum overload. + const NativeCanvasPath2D* path = nullptr; if (info.Length() >= 1 && !info[0].IsUndefined()) { path = NativeCanvasPath2D::TryUnwrap(info.Env(), info[0]); - if (path == nullptr && !info[0].IsString()) + if (path == nullptr) { - throw Napi::TypeError::New(info.Env(), "Context2D.fill: the first argument is neither a Path2D nor a fill rule."); + ValidateFillRule(info.Env(), info[0]); } } + if (path != nullptr && info.Length() >= 2 && !info[1].IsUndefined()) + { + ValidateFillRule(info.Env(), info[1]); + } // draw Path2D if exists if (path != nullptr) From 771b6fa8346a274f8c60cf4d2928b9c514ede3b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Mon, 14 Sep 2026 14:18:26 -0700 Subject: [PATCH 11/12] Canvas: define native brands portably Use Object.defineProperty for the temporary native-instance brand so the implementation works on the JSI port, which exposes neither the C napi_property_descriptor API nor Napi::PropertyDescriptor. Preserve the non-enumerable, non-configurable, non-writable descriptor semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 54aaa1e1-b4b3-46e7-9de4-5a56add4ac42 --- .../Canvas/Source/NativeInstanceRegistry.h | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/Polyfills/Canvas/Source/NativeInstanceRegistry.h b/Polyfills/Canvas/Source/NativeInstanceRegistry.h index 17a516f48b..ae58741ca0 100644 --- a/Polyfills/Canvas/Source/NativeInstanceRegistry.h +++ b/Polyfills/Canvas/Source/NativeInstanceRegistry.h @@ -18,24 +18,19 @@ namespace Babylon::Polyfills::Internal static void Add(const Napi::CallbackInfo& info, T* instance) { // Non-enumerable brand so it does not show up in Object.keys / for..in. - // Use the C napi_define_properties path: Napi::PropertyDescriptor is not - // available on every JsRuntimeHost port (notably UWP/JSI). - Napi::Object self = info.This().As(); - Napi::External brand = Napi::External::New(info.Env(), instance); - napi_property_descriptor desc{}; - desc.utf8name = BRAND_NAME; - desc.value = brand; - desc.attributes = napi_default; // non-enumerable, non-configurable, non-writable - napi_status status = napi_define_properties(info.Env(), self, 1, &desc); - if (status != napi_ok) - { - Napi::Error::New(info.Env(), "NativeInstanceRegistry: failed to brand instance").ThrowAsJavaScriptException(); - return; - } + // Object.defineProperty is supported by every JsRuntimeHost port, unlike the + // Napi::PropertyDescriptor and C Node-API surfaces. Omitted flags default to false. + Napi::Object self = info.This().As(); + Napi::Object descriptor = Napi::Object::New(info.Env()); + descriptor.Set("value", Napi::External::New(info.Env(), instance)); + Napi::Object object = info.Env().Global().Get("Object").As(); + object.Get("defineProperty").As().Call( + object, + {self, Napi::String::New(info.Env(), BRAND_NAME), descriptor}); - const std::scoped_lock lock{Mutex()}; - Instances().insert(instance); - } + const std::scoped_lock lock{Mutex()}; + Instances().insert(instance); + } static void Remove(const T* instance) { From 0c28cb8ff914f80112e01abe7e758cb11fa3f7fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Branimir=20Karad=C5=BEi=C4=87?= Date: Wed, 16 Sep 2026 08:42:25 -0700 Subject: [PATCH 12/12] Canvas: brand native Canvas sources before drawImage Use the shared native instance registry for Canvas so drawImage classifies real Canvas sources before coordinate coercion without relying on forgeable instanceof checks or unchecked ObjectWrap::Unwrap. Update the rebased branding test for upstream Canvas-as-source support and cover a Canvas prototype impostor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48fa7ec8-0cf6-48a0-b836-10b7ace38c1a --- .../JavaScript/dist/tests.javaScript.all.js | 13 ++++++++----- .../JavaScript/src/tests.javaScript.all.ts | 13 ++++++++----- Polyfills/Canvas/Source/Canvas.cpp | 9 +++++++++ Polyfills/Canvas/Source/Canvas.h | 2 ++ Polyfills/Canvas/Source/Context.cpp | 5 +---- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js index aed5e5d609..da059e5559 100644 --- a/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js +++ b/Apps/UnitTests/JavaScript/dist/tests.javaScript.all.js @@ -29149,7 +29149,7 @@ describe("Canvas2D", function () { (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill("evenodd");}).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(new String("evenodd"));}).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () { - ctx.fill({ toString: function () {return "nonzero";} }); + ctx.fill({ toString: function toString() {return "nonzero";} }); }).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path);}).to.not.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.fill(path, "nonzero");}).to.not.throw(); @@ -29170,13 +29170,16 @@ describe("Canvas2D", function () { (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {path.addPath(new Path2D("M0 0 L5 5"));}).to.not.throw(); }); - it("rejects a non-Image argument to drawImage", function () { - // Non-Image sources must throw, not AV via Unwrap. + it("rejects an invalid native source argument to drawImage", function () { + // Native-looking impostors must throw, not AV via an unchecked Unwrap. var ctx = createContext(); - var otherCanvas = new _native.Canvas(); + var realCanvas = new _native.Canvas(); + var spoofedCanvas = Object.create(Object.getPrototypeOf(realCanvas)); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(spoofedCanvas instanceof _native.Canvas).to.equal(true); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage({}, 0, 0);}).to.throw(); (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage(new Path2D(), 0, 0);}).to.throw(); - (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage(otherCanvas, 0, 0);}).to.throw(); + (0,chai__WEBPACK_IMPORTED_MODULE_3__.expect)(function () {ctx.drawImage(spoofedCanvas, 0, 0);}).to.throw(); + realCanvas.dispose(); }); it("treats a non-Path2D Path2D() argument as path data", function () { diff --git a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts index 72699ec9dd..8b451d90df 100644 --- a/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts +++ b/Apps/UnitTests/JavaScript/src/tests.javaScript.all.ts @@ -668,13 +668,16 @@ describe("Canvas2D", function () { expect(function () { path.addPath(new Path2D("M0 0 L5 5")); }).to.not.throw(); }); - it("rejects a non-Image argument to drawImage", function () { - // Non-Image sources must throw, not AV via Unwrap. - const ctx = createContext(); - const otherCanvas = new _native.Canvas(); + it("rejects an invalid native source argument to drawImage", function () { + // Native-looking impostors must throw, not AV via an unchecked Unwrap. + const ctx = createContext(); + const realCanvas = new _native.Canvas(); + const spoofedCanvas = Object.create(Object.getPrototypeOf(realCanvas)); + expect(spoofedCanvas instanceof _native.Canvas).to.equal(true); expect(function () { ctx.drawImage({}, 0, 0); }).to.throw(); expect(function () { ctx.drawImage(new Path2D(), 0, 0); }).to.throw(); - expect(function () { ctx.drawImage(otherCanvas, 0, 0); }).to.throw(); + expect(function () { ctx.drawImage(spoofedCanvas, 0, 0); }).to.throw(); + realCanvas.dispose(); }); it("treats a non-Path2D Path2D() argument as path data", function () { diff --git a/Polyfills/Canvas/Source/Canvas.cpp b/Polyfills/Canvas/Source/Canvas.cpp index e08b443d16..4df3fbfd46 100644 --- a/Polyfills/Canvas/Source/Canvas.cpp +++ b/Polyfills/Canvas/Source/Canvas.cpp @@ -11,6 +11,7 @@ #include "Colors.h" #include "Gradient.h" #include "Font.h" +#include "NativeInstanceRegistry.h" #include #include #include @@ -47,15 +48,23 @@ namespace Babylon::Polyfills::Internal JsRuntime::NativeObject::GetFromJavaScript(env).Set(JS_CONSTRUCTOR_NAME, func); } + NativeCanvas* NativeCanvas::TryUnwrap(Napi::Env env, const Napi::Value& value) + { + return NativeInstanceRegistry::TryUnwrap(env, value); + } + NativeCanvas::NativeCanvas(const Napi::CallbackInfo& info) : Napi::ObjectWrap{info} , m_graphicsContext{Graphics::DeviceContext::GetFromJavaScript(info.Env())} , Polyfills::Canvas::Impl::MonitoredResource{Polyfills::Canvas::Impl::GetFromJavaScript(info.Env())} { + NativeInstanceRegistry::Add(info, this); } NativeCanvas::~NativeCanvas() { + NativeInstanceRegistry::Remove(this); + // Canvas and Context form a JS cycle; finalizer order is not guaranteed. // Clear the reverse pointer first so Context::~Context cannot touch us. if (m_context != nullptr) diff --git a/Polyfills/Canvas/Source/Canvas.h b/Polyfills/Canvas/Source/Canvas.h index 6cc504a90f..6d4d6e792b 100644 --- a/Polyfills/Canvas/Source/Canvas.h +++ b/Polyfills/Canvas/Source/Canvas.h @@ -61,6 +61,8 @@ namespace Babylon::Polyfills::Internal public: static void Initialize(Napi::Env env); + static NativeCanvas* TryUnwrap(Napi::Env env, const Napi::Value& value); + explicit NativeCanvas(const Napi::CallbackInfo& info); virtual ~NativeCanvas(); diff --git a/Polyfills/Canvas/Source/Context.cpp b/Polyfills/Canvas/Source/Context.cpp index 71899a2d42..e65fba0909 100644 --- a/Polyfills/Canvas/Source/Context.cpp +++ b/Polyfills/Canvas/Source/Context.cpp @@ -1293,10 +1293,7 @@ namespace Babylon::Polyfills::Internal Napi::Object imageObj = info[0].As(); // Retain the source kind before coercion can change its prototype or properties. // Canvas takes precedence over the structural ImageBitmap shape. - const auto canvasCtorVal = JsRuntime::NativeObject::GetFromJavaScript(info.Env()).Get("Canvas"); - NativeCanvas* const srcCanvas = canvasCtorVal.IsFunction() && imageObj.InstanceOf(canvasCtorVal.As()) - ? NativeCanvas::Unwrap(imageObj) - : nullptr; + NativeCanvas* const srcCanvas = NativeCanvas::TryUnwrap(info.Env(), imageObj); const NativeCanvasImage* const canvasImage = srcCanvas == nullptr ? NativeCanvasImage::TryUnwrap(info.Env(), imageObj) : nullptr;