From cf0b043b1c2afa81522eba8018679e1abb8b63e3 Mon Sep 17 00:00:00 2001 From: Erik Corry Date: Wed, 21 Jan 2026 20:27:47 +0100 Subject: [PATCH 1/5] This() deprecation branch --- src/workerd/jsg/resource.h | 35 +++++++-- v8-this-deprecation-case.md | 144 ++++++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 v8-this-deprecation-case.md diff --git a/src/workerd/jsg/resource.h b/src/workerd/jsg/resource.h index 3d5abe0ddd9..759496d9189 100644 --- a/src/workerd/jsg/resource.h +++ b/src/workerd/jsg/resource.h @@ -639,12 +639,19 @@ struct GetterCallback; liftKj(info, [&]() { \ auto isolate = info.GetIsolate(); \ auto context = isolate->GetCurrentContext(); \ + // For interceptors on the prototype, HolderV2() returns the prototype, but This() \ + // returns the actual instance. We need to validate This() for security, but use \ + // HolderV2() wouldn't work here since it's the prototype object. \ + // Note: This still uses the deprecated This() API - see V8 bug 455600234. \ auto obj = info.This(); \ auto& js = Lock::from(isolate); \ auto& wrapper = TypeWrapper::from(isolate); \ /* V8 no longer supports AccessorSignature, so we must manually verify `this`'s type. */ \ + /* Also check that HolderV2() == This() to prevent "prototype-as-this" attacks where */ \ + /* a native object is used as a prototype and accessed through a derived object. */ \ if (!isContext && \ - !wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj)) { \ + (!wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj) || \ + obj != info.This())) { \ throwTypeError(isolate, kIllegalInvocation); \ } \ auto& self = extractInternalPointer(context, obj); \ @@ -689,8 +696,11 @@ struct GetterCallback; auto obj = info.This(); \ auto& wrapper = TypeWrapper::from(isolate); \ /* V8 no longer supports AccessorSignature, so we must manually verify `this`'s type. */ \ + /* Also check that HolderV2() == This() to prevent "prototype-as-this" attacks where */ \ + /* a native object is used as a prototype and accessed through a derived object. */ \ if (!isContext && \ - !wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj)) { \ + (!wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj) || \ + obj != info.This())) { \ throwTypeError(isolate, kIllegalInvocation); \ } \ auto& self = extractInternalPointer(context, obj); \ @@ -889,7 +899,11 @@ struct SetterCallback(nullptr))->HasInstance(obj)) { + // Also check that HolderV2() == This() to prevent "prototype-as-this" attacks where + // a native object is used as a prototype and accessed through a derived object. + if (!isContext && + (!wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj) || + obj != info.This())) { throwTypeError(isolate, kIllegalInvocation); } auto& self = extractInternalPointer(context, obj); @@ -915,7 +929,11 @@ struct SetterCallback(nullptr))->HasInstance(obj)) { + // Also check that HolderV2() == This() to prevent "prototype-as-this" attacks where + // a native object is used as a prototype and accessed through a derived object. + if (!isContext && + (!wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj) || + obj != info.This())) { throwTypeError(isolate, kIllegalInvocation); } auto& self = extractInternalPointer(context, obj); @@ -1449,7 +1467,11 @@ struct ResourceTypeBuilder { template inline void registerInspectProperty() { - using Gcb = GetterCallback; + // Use PropertyGetterCallback (FunctionCallbackInfo) instead of GetterCallback + // (PropertyCallbackInfo) because this property is on the prototype but needs access + // to instance state. FunctionCallbackInfo::This() is not deprecated, while + // PropertyCallbackInfo::This() is. + using Gcb = PropertyGetterCallback; auto v8Name = v8StrIntern(isolate, name); @@ -1457,7 +1479,8 @@ struct ResourceTypeBuilder { auto symbol = v8::Symbol::New(isolate, v8Name); inspectProperties->Set(v8Name, symbol, v8::PropertyAttribute::ReadOnly); - prototype->SetNativeDataProperty(symbol, &Gcb::callback, nullptr, v8::Local(), + auto getterFn = v8::FunctionTemplate::New(isolate, &Gcb::callback); + prototype->SetAccessorProperty(symbol, getterFn, v8::Local(), static_cast( v8::PropertyAttribute::ReadOnly | v8::PropertyAttribute::DontEnum)); } diff --git a/v8-this-deprecation-case.md b/v8-this-deprecation-case.md new file mode 100644 index 00000000000..48956e245ce --- /dev/null +++ b/v8-this-deprecation-case.md @@ -0,0 +1,144 @@ +# Case for Un-deprecating PropertyCallbackInfo::This() + +## Summary + +The deprecation of `PropertyCallbackInfo::This()` in favor of `HolderV2()` creates an **unsolvable security vulnerability** for embedders that cannot be mitigated without breaking backwards compatibility. We request that V8 either: + +1. Un-deprecate `This()` for property callbacks, or +2. Provide an alternative API that gives access to the actual receiver + +## The Security Problem + +When `HolderV2()` is used instead of `This()`, embedders cannot distinguish between: + +```javascript +// Legitimate access +const req = new Request("https://api.example.com", { + headers: { "Authorization": "Bearer secret-token" } +}); +req.headers.get("Authorization"); // Should work +``` + +and: + +```javascript +// Prototype-as-this attack +function EvilRequest() {} +EvilRequest.prototype = new Request("https://api.example.com", { + headers: { "Authorization": "Bearer secret-token" } +}); +const evil = new EvilRequest(); +evil.headers; // With HolderV2() only, this returns the secret headers! +``` + +With `This()`: The embedder sees `evil` (an EvilRequest), fails `HasInstance`, rejects. +With `HolderV2()`: The embedder sees the Request from the prototype chain, passes `HasInstance`, **leaks data**. + +## Why This Matters for Cloudflare Workers + +Cloudflare Workers runs untrusted JavaScript from millions of customers in a shared environment. Security boundaries are critical. + +### Real APIs Affected + +**1. Request/Response Objects** +```javascript +// Attack: Extract body/headers from a Request used as prototype +function Wrapper() {} +Wrapper.prototype = realRequest; // Request with sensitive headers +new Wrapper().headers; // Leaks Authorization, Cookie, etc. +``` + +**2. Streams (ReadableStream/WritableStream)** +```javascript +// Attack: Access internal stream state +function FakeStream() {} +FakeStream.prototype = realReadableStream; +new FakeStream().locked; // Accesses internal state of real stream +``` + +**3. WebSocket** +```javascript +// Attack: Access WebSocket properties +function FakeSocket() {} +FakeSocket.prototype = realWebSocket; +new FakeSocket().url; // Leaks the WebSocket URL +new FakeSocket().readyState; // Leaks connection state +``` + +**4. Crypto Keys (SubtleCrypto)** +```javascript +// Attack: Potentially extract key material +function FakeKey() {} +FakeKey.prototype = realCryptoKey; +new FakeKey().algorithm; // Leaks key algorithm details +``` + +### The Backwards Compatibility Constraint + +Cloudflare Workers has **strict backwards compatibility requirements**. Code deployed years ago must continue to work identically. We cannot: + +- Change property semantics (data vs accessor) +- Change prototype chain behavior +- Add new error conditions that weren't there before + +The *only* viable solution is to detect the attack at the native layer using `This()`. + +## Why HolderV2() Is Insufficient + +| Scenario | This() | HolderV2() | Correct Behavior | +|----------|--------|------------|------------------| +| Direct access: `obj.prop` | obj | obj | Allow | +| Prototype attack: `derived.prop` | derived | prototypeObj | **Reject** | +| Reflect.get: `Reflect.get(obj, 'prop', receiver)` | receiver | obj | Depends | + +`HolderV2()` cannot distinguish case 1 from case 2. Only `This()` can. + +## The Interceptor Problem + +For interceptors (`SetHandler`), the situation is worse: + +- Interceptors are registered on the **prototype** +- `HolderV2()` returns the prototype object itself +- `This()` returns the actual instance + +Without `This()`, interceptors cannot access the correct object at all, not just for security—for basic functionality. + +```cpp +// Current workerd code - REQUIRES This() +static v8::Intercepted getter(..., const v8::PropertyCallbackInfo& info) { + auto obj = info.This(); // Need the actual instance, not the prototype + auto& self = extractInternalPointer(context, obj); + // ... access instance state +} +``` + +## Proposed Solutions + +### Option A: Un-deprecate This() +The simplest solution. `This()` provides essential information that `HolderV2()` cannot. + +### Option B: Add a New API +```cpp +// Something like: +v8::Local PropertyCallbackInfo::Receiver() const; +``` +That explicitly provides the receiver for security checks. + +### Option C: Add a Flag/Mode +Allow embedders to opt-in to receiving `This()` semantics when they need it for security. + +## Conclusion + +The deprecation of `This()` forces embedders to choose between: + +1. **Security vulnerability** - Use `HolderV2()` and allow prototype-as-this attacks +2. **Breaking changes** - Restructure APIs in backwards-incompatible ways +3. **Ignore deprecation** - Continue using `This()` despite warnings + +None of these are acceptable for a production runtime serving millions of users. + +We respectfully request that the V8 team reconsider this deprecation and provide a path forward that preserves both security and compatibility. + +--- + +*Contact: Erik Corry, Cloudflare* From 919565b7ddb5ac7aac5aaeb10fa528943356f9e5 Mon Sep 17 00:00:00 2001 From: Erik Corry Date: Wed, 21 Jan 2026 20:52:40 +0100 Subject: [PATCH 2/5] Updated .md --- v8-this-deprecation-case.md | 182 ++++++++++++++++++------------------ 1 file changed, 91 insertions(+), 91 deletions(-) diff --git a/v8-this-deprecation-case.md b/v8-this-deprecation-case.md index 48956e245ce..1746e0dc785 100644 --- a/v8-this-deprecation-case.md +++ b/v8-this-deprecation-case.md @@ -1,144 +1,144 @@ -# Case for Un-deprecating PropertyCallbackInfo::This() +# Case for PropertyCallbackInfo::This() (or Equivalent) ## Summary -The deprecation of `PropertyCallbackInfo::This()` in favor of `HolderV2()` creates an **unsolvable security vulnerability** for embedders that cannot be mitigated without breaking backwards compatibility. We request that V8 either: +Cloudflare Workers needs `PropertyCallbackInfo::This()` (or an equivalent API) to support Workers deployed before 2022-01-31 that use instance-level properties. Without this, we cannot maintain backwards compatibility for these Workers. -1. Un-deprecate `This()` for property callbacks, or -2. Provide an alternative API that gives access to the actual receiver +## Background: How Workerd Handles Properties -## The Security Problem +Workerd has two patterns for exposing properties: -When `HolderV2()` is used instead of `This()`, embedders cannot distinguish between: +**1. Prototype Properties (modern, post-2022-01-31)** +```cpp +// Uses SetAccessorProperty with FunctionTemplate +auto getterFn = v8::FunctionTemplate::New(isolate, Gcb::callback); +prototype->SetAccessorProperty(v8Name, getterFn, ...); -```javascript -// Legitimate access -const req = new Request("https://api.example.com", { - headers: { "Authorization": "Bearer secret-token" } -}); -req.headers.get("Authorization"); // Should work +// Callback uses FunctionCallbackInfo::This() - NOT deprecated +void callback(const v8::FunctionCallbackInfo& info) { + auto obj = info.This(); // OK - not deprecated +} ``` -and: +**2. Instance Properties (legacy, pre-2022-01-31)** +```cpp +// Uses SetNativeDataProperty on instance template +instance->SetNativeDataProperty(v8Name, &Gcb::callback, ...); -```javascript -// Prototype-as-this attack -function EvilRequest() {} -EvilRequest.prototype = new Request("https://api.example.com", { - headers: { "Authorization": "Bearer secret-token" } -}); -const evil = new EvilRequest(); -evil.headers; // With HolderV2() only, this returns the secret headers! +// Callback uses PropertyCallbackInfo::This() - DEPRECATED +void callback(v8::Local, const v8::PropertyCallbackInfo& info) { + auto obj = info.This(); // Deprecated! +} ``` -With `This()`: The embedder sees `evil` (an EvilRequest), fails `HasInstance`, rejects. -With `HolderV2()`: The embedder sees the Request from the prototype chain, passes `HasInstance`, **leaks data**. +## The Compatibility Flag + +In January 2022, we migrated properties from instance to prototype: + +```capnp +jsgPropertyOnPrototypeTemplate @7 :Bool + $compatEnableFlag("workers_api_getters_setters_on_prototype") + $compatEnableDate("2022-01-31") +``` -## Why This Matters for Cloudflare Workers +**Workers with compatibility date >= 2022-01-31**: Use prototype properties, `FunctionCallbackInfo::This()` - no problem. -Cloudflare Workers runs untrusted JavaScript from millions of customers in a shared environment. Security boundaries are critical. +**Workers with compatibility date < 2022-01-31**: Use instance properties, `PropertyCallbackInfo::This()` - affected by deprecation. -### Real APIs Affected +## Why We Can't Just Migrate Old Workers -**1. Request/Response Objects** +The change from instance to prototype properties is **not purely internal**. It has observable behavioral differences: + +### 1. Subclassing Behavior ```javascript -// Attack: Extract body/headers from a Request used as prototype -function Wrapper() {} -Wrapper.prototype = realRequest; // Request with sensitive headers -new Wrapper().headers; // Leaks Authorization, Cookie, etc. +class MyRequest extends Request { + get headers() { + return "overridden"; + } +} +const r = new MyRequest("https://example.com"); +r.headers; // Instance: "overridden" (own property shadows) + // Prototype: "overridden" (prototype chain) ``` +This one happens to work the same, but... -**2. Streams (ReadableStream/WritableStream)** +### 2. Property Enumeration ```javascript -// Attack: Access internal stream state -function FakeStream() {} -FakeStream.prototype = realReadableStream; -new FakeStream().locked; // Accesses internal state of real stream +const r = new Request("https://example.com"); +Object.keys(r); // Instance: ["headers", "url", ...] + // Prototype: [] +Object.getOwnPropertyNames(r); // Instance: ["headers", "url", ...] + // Prototype: [] ``` -**3. WebSocket** +### 3. hasOwnProperty ```javascript -// Attack: Access WebSocket properties -function FakeSocket() {} -FakeSocket.prototype = realWebSocket; -new FakeSocket().url; // Leaks the WebSocket URL -new FakeSocket().readyState; // Leaks connection state +const r = new Request("https://example.com"); +r.hasOwnProperty("headers"); // Instance: true + // Prototype: false ``` -**4. Crypto Keys (SubtleCrypto)** +### 4. Property Descriptors ```javascript -// Attack: Potentially extract key material -function FakeKey() {} -FakeKey.prototype = realCryptoKey; -new FakeKey().algorithm; // Leaks key algorithm details +Object.getOwnPropertyDescriptor(r, "headers"); +// Instance: {value: ..., writable: false, enumerable: true, configurable: false} +// Prototype: undefined (property is on prototype, not instance) ``` -### The Backwards Compatibility Constraint +Any Worker code that depends on these behaviors would break if we forcibly migrated them. -Cloudflare Workers has **strict backwards compatibility requirements**. Code deployed years ago must continue to work identically. We cannot: +## Why HolderV2() Doesn't Work for Instance Properties -- Change property semantics (data vs accessor) -- Change prototype chain behavior -- Add new error conditions that weren't there before +For instance properties via `SetNativeDataProperty`, when someone uses a native object as a prototype: -The *only* viable solution is to detect the attack at the native layer using `This()`. - -## Why HolderV2() Is Insufficient +```javascript +function Evil() {} +Evil.prototype = new Request("https://example.com"); +new Evil().headers; // Should throw TypeError +``` -| Scenario | This() | HolderV2() | Correct Behavior | -|----------|--------|------------|------------------| -| Direct access: `obj.prop` | obj | obj | Allow | -| Prototype attack: `derived.prop` | derived | prototypeObj | **Reject** | -| Reflect.get: `Reflect.get(obj, 'prop', receiver)` | receiver | obj | Depends | +- `This()` returns the `Evil` instance → `HasInstance` check fails → correctly throws +- `HolderV2()` returns the `Request` from prototype chain → `HasInstance` passes → **incorrectly succeeds** -`HolderV2()` cannot distinguish case 1 from case 2. Only `This()` can. +We can add `HolderV2() == This()` check, but this still requires `This()`. -## The Interceptor Problem +## Named Property Interceptors -For interceptors (`SetHandler`), the situation is worse: +For interceptors (`SetHandler` on prototype), the situation is different: -- Interceptors are registered on the **prototype** -- `HolderV2()` returns the prototype object itself -- `This()` returns the actual instance +- `HolderV2()` returns the **prototype object** (where the handler is installed) +- `This()` returns the **actual instance** -Without `This()`, interceptors cannot access the correct object at all, not just for security—for basic functionality. +Without `This()`, interceptors cannot access the correct object at all. This isn't a security issue—it's "the API doesn't work." +Chromium handles this with explicit checks: ```cpp -// Current workerd code - REQUIRES This() -static v8::Intercepted getter(..., const v8::PropertyCallbackInfo& info) { - auto obj = info.This(); // Need the actual instance, not the prototype - auto& self = extractInternalPointer(context, obj); - // ... access instance state +// WebIDL spec: https://webidl.spec.whatwg.org/#legacy-platform-object-set +if (info.Holder() == info.This()) { + // Only proceed if they match } ``` -## Proposed Solutions +## What We Need -### Option A: Un-deprecate This() -The simplest solution. `This()` provides essential information that `HolderV2()` cannot. +One of: -### Option B: Add a New API -```cpp -// Something like: -v8::Local PropertyCallbackInfo::Receiver() const; -``` -That explicitly provides the receiver for security checks. +1. **Keep `PropertyCallbackInfo::This()` available** (even if "discouraged") + +2. **Add `PropertyCallbackInfo::Receiver()`** - a new method that returns the receiver, for cases where `HolderV2()` is insufficient -### Option C: Add a Flag/Mode -Allow embedders to opt-in to receiving `This()` semantics when they need it for security. +3. **Document the migration path clearly** - if V8's position is "use `SetAccessorProperty` with `FunctionTemplate`", document that this is the only supported pattern going forward -## Conclusion +## Questions for V8 Team -The deprecation of `This()` forces embedders to choose between: +1. What is Chromium's plan for `SetNativeDataProperty` callbacks? Are they migrating everything to `SetAccessorProperty`? -1. **Security vulnerability** - Use `HolderV2()` and allow prototype-as-this attacks -2. **Breaking changes** - Restructure APIs in backwards-incompatible ways -3. **Ignore deprecation** - Continue using `This()` despite warnings +2. For named property interceptors, what's the recommended way to get the actual receiver? -None of these are acceptable for a production runtime serving millions of users. +3. Is there a timeline for when `This()` will actually be removed (vs just deprecated)? -We respectfully request that the V8 team reconsider this deprecation and provide a path forward that preserves both security and compatibility. +4. For embedders with backwards compatibility requirements, what's the recommended approach? --- -*Contact: Erik Corry, Cloudflare* +*Erik Corry, Cloudflare* From 057522a7e0753ad72a1699739e9bbde4271b1eaa Mon Sep 17 00:00:00 2001 From: Erik Corry Date: Wed, 21 Jan 2026 21:26:39 +0100 Subject: [PATCH 3/5] fix registerWildcardProperty by calling SetHandler on the instance instead of the prototype --- src/workerd/jsg/resource.h | 9 ++- v8-this-deprecation-case.md | 147 +++++++++++------------------------- 2 files changed, 52 insertions(+), 104 deletions(-) diff --git a/src/workerd/jsg/resource.h b/src/workerd/jsg/resource.h index 759496d9189..c89f61bb241 100644 --- a/src/workerd/jsg/resource.h +++ b/src/workerd/jsg/resource.h @@ -1217,7 +1217,9 @@ struct WildcardPropertyCallbacks v8::Local { auto isolate = info.GetIsolate(); auto context = isolate->GetCurrentContext(); - auto obj = info.This(); + // With the interceptor on the instance template, HolderV2() returns the instance + // (same as This()), so we can use the non-deprecated HolderV2() API. + auto obj = info.HolderV2(); auto& wrapper = TypeWrapper::from(isolate); if (!wrapper.getTemplate(isolate, static_cast(nullptr))->HasInstance(obj)) { throwTypeError(isolate, kIllegalInvocation); @@ -1267,7 +1269,10 @@ struct ResourceTypeBuilder { template inline void registerWildcardProperty() { - prototype->SetHandler( + // Install on instance template (not prototype) so that HolderV2() == This(). + // This matches Chromium's approach and avoids needing the deprecated + // PropertyCallbackInfo::This(). + instance->SetHandler( WildcardPropertyCallbacks{}); } diff --git a/v8-this-deprecation-case.md b/v8-this-deprecation-case.md index 1746e0dc785..e5d92384c84 100644 --- a/v8-this-deprecation-case.md +++ b/v8-this-deprecation-case.md @@ -2,142 +2,85 @@ ## Summary -Cloudflare Workers needs `PropertyCallbackInfo::This()` (or an equivalent API) to support Workers deployed before 2022-01-31 that use instance-level properties. Without this, we cannot maintain backwards compatibility for these Workers. +After investigation, we found that **most uses of `PropertyCallbackInfo::This()` in workerd can be eliminated** by aligning with Chromium's patterns. However, there's one remaining case for legacy Workers. -## Background: How Workerd Handles Properties +## What We Found -Workerd has two patterns for exposing properties: +### 1. Prototype Properties (SOLVED) + +Workerd already migrated to `SetAccessorProperty` with `FunctionTemplate` for prototype properties in 2022: -**1. Prototype Properties (modern, post-2022-01-31)** ```cpp -// Uses SetAccessorProperty with FunctionTemplate auto getterFn = v8::FunctionTemplate::New(isolate, Gcb::callback); prototype->SetAccessorProperty(v8Name, getterFn, ...); - -// Callback uses FunctionCallbackInfo::This() - NOT deprecated -void callback(const v8::FunctionCallbackInfo& info) { - auto obj = info.This(); // OK - not deprecated -} ``` -**2. Instance Properties (legacy, pre-2022-01-31)** -```cpp -// Uses SetNativeDataProperty on instance template -instance->SetNativeDataProperty(v8Name, &Gcb::callback, ...); - -// Callback uses PropertyCallbackInfo::This() - DEPRECATED -void callback(v8::Local, const v8::PropertyCallbackInfo& info) { - auto obj = info.This(); // Deprecated! -} -``` +This uses `FunctionCallbackInfo::This()` which is **NOT deprecated**. Workers with compatibility date >= 2022-01-31 use this pattern. -## The Compatibility Flag +### 2. Named Property Interceptors (CAN BE SOLVED) -In January 2022, we migrated properties from instance to prototype: +We discovered workerd puts interceptors on the **prototype**, while Chromium puts them on the **instance template**: -```capnp -jsgPropertyOnPrototypeTemplate @7 :Bool - $compatEnableFlag("workers_api_getters_setters_on_prototype") - $compatEnableDate("2022-01-31") +```python +# Chromium (interface.py line 5815) +interceptor_template = "${instance_template}" # Not prototype! ``` -**Workers with compatibility date >= 2022-01-31**: Use prototype properties, `FunctionCallbackInfo::This()` - no problem. - -**Workers with compatibility date < 2022-01-31**: Use instance properties, `PropertyCallbackInfo::This()` - affected by deprecation. - -## Why We Can't Just Migrate Old Workers - -The change from instance to prototype properties is **not purely internal**. It has observable behavioral differences: - -### 1. Subclassing Behavior -```javascript -class MyRequest extends Request { - get headers() { - return "overridden"; - } -} -const r = new MyRequest("https://example.com"); -r.headers; // Instance: "overridden" (own property shadows) - // Prototype: "overridden" (prototype chain) -``` -This one happens to work the same, but... - -### 2. Property Enumeration -```javascript -const r = new Request("https://example.com"); -Object.keys(r); // Instance: ["headers", "url", ...] - // Prototype: [] -Object.getOwnPropertyNames(r); // Instance: ["headers", "url", ...] - // Prototype: [] -``` +When interceptor is on instance: `HolderV2() == This()` → can use `HolderV2()` +When interceptor is on prototype: `HolderV2() != This()` → need `This()` -### 3. hasOwnProperty -```javascript -const r = new Request("https://example.com"); -r.hasOwnProperty("headers"); // Instance: true - // Prototype: false -``` - -### 4. Property Descriptors -```javascript -Object.getOwnPropertyDescriptor(r, "headers"); -// Instance: {value: ..., writable: false, enumerable: true, configurable: false} -// Prototype: undefined (property is on prototype, not instance) +**Fix**: Change `registerWildcardProperty()` from: +```cpp +prototype->SetHandler(...) ``` - -Any Worker code that depends on these behaviors would break if we forcibly migrated them. - -## Why HolderV2() Doesn't Work for Instance Properties - -For instance properties via `SetNativeDataProperty`, when someone uses a native object as a prototype: - -```javascript -function Evil() {} -Evil.prototype = new Request("https://example.com"); -new Evil().headers; // Should throw TypeError +to: +```cpp +instance->SetHandler(...) ``` -- `This()` returns the `Evil` instance → `HasInstance` check fails → correctly throws -- `HolderV2()` returns the `Request` from prototype chain → `HasInstance` passes → **incorrectly succeeds** - -We can add `HolderV2() == This()` check, but this still requires `This()`. - -## Named Property Interceptors +This matches Chromium and eliminates the need for `This()` in interceptors. See `interceptor-on-instance.patch`. -For interceptors (`SetHandler` on prototype), the situation is different: +### 3. Legacy Instance Properties (NEEDS SOLUTION) -- `HolderV2()` returns the **prototype object** (where the handler is installed) -- `This()` returns the **actual instance** +Workers with compatibility date < 2022-01-31 use instance properties via `SetNativeDataProperty`. These need `This()` to detect "prototype-as-this" attacks: -Without `This()`, interceptors cannot access the correct object at all. This isn't a security issue—it's "the API doesn't work." - -Chromium handles this with explicit checks: ```cpp -// WebIDL spec: https://webidl.spec.whatwg.org/#legacy-platform-object-set -if (info.Holder() == info.This()) { - // Only proceed if they match +auto obj = info.HolderV2(); +if (obj != info.This()) { // Still need This() for this check + throwTypeError(isolate, kIllegalInvocation); } ``` -## What We Need +## Observable Differences -One of: +Moving interceptor from prototype to instance may affect: -1. **Keep `PropertyCallbackInfo::This()` available** (even if "discouraged") +| Operation | Prototype Interceptor | Instance Interceptor | +|-----------|----------------------|---------------------| +| `hasOwnProperty('x')` | Query callback NOT called | Query callback called (if defined) | +| `Object.keys(obj)` | Enumerator on prototype | Enumerator on instance | -2. **Add `PropertyCallbackInfo::Receiver()`** - a new method that returns the receiver, for cases where `HolderV2()` is insufficient +However, workerd uses `kNonMasking` and has no query callback, so impact may be minimal. -3. **Document the migration path clearly** - if V8's position is "use `SetAccessorProperty` with `FunctionTemplate`", document that this is the only supported pattern going forward +V8 test `InterceptorHasOwnProperty` (test-api-interceptors.cc:1071) confirms: +- With interceptor on instance: `hasOwnProperty` returns false for non-intercepted properties +- After setting the property: `hasOwnProperty` returns true ## Questions for V8 Team -1. What is Chromium's plan for `SetNativeDataProperty` callbacks? Are they migrating everything to `SetAccessorProperty`? +1. **For the legacy instance property case**: Is there a recommended alternative to `This()` for detecting when an object is accessed through the prototype chain vs directly? + +2. **For interceptors**: V8 bug 455600234 says "For interceptors, using This() is always semantically correct." Does this mean `This()` will remain available for interceptors even after deprecation? -2. For named property interceptors, what's the recommended way to get the actual receiver? +3. **Timeline**: When will `This()` actually be removed vs just deprecated with warnings? -3. Is there a timeline for when `This()` will actually be removed (vs just deprecated)? +## Summary -4. For embedders with backwards compatibility requirements, what's the recommended approach? +| Use Case | Current Status | Solution | +|----------|---------------|----------| +| Prototype properties (new Workers) | Uses `FunctionCallbackInfo::This()` | Already solved | +| Named interceptors | Uses `PropertyCallbackInfo::This()` | Move to instance template | +| Instance properties (legacy Workers) | Uses `PropertyCallbackInfo::This()` | Needs `This()` or alternative | --- From bdd8c308e686fb8b415e071081f2dc816f4ba718 Mon Sep 17 00:00:00 2001 From: Erik Corry Date: Wed, 21 Jan 2026 21:44:46 +0100 Subject: [PATCH 4/5] explain how RPC breaks --- v8-this-deprecation-case.md | 102 ++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/v8-this-deprecation-case.md b/v8-this-deprecation-case.md index e5d92384c84..1bab3d4e115 100644 --- a/v8-this-deprecation-case.md +++ b/v8-this-deprecation-case.md @@ -1,86 +1,84 @@ -# Case for PropertyCallbackInfo::This() (or Equivalent) +# Case for PropertyCallbackInfo::This() ## Summary -After investigation, we found that **most uses of `PropertyCallbackInfo::This()` in workerd can be eliminated** by aligning with Chromium's patterns. However, there's one remaining case for legacy Workers. +Cloudflare Workers (workerd) genuinely needs `PropertyCallbackInfo::This()` and **cannot follow Chromium's pattern**. We tested moving interceptors from prototype to instance template (like Chromium does) and it breaks RPC functionality. -## What We Found +## What We Tested -### 1. Prototype Properties (SOLVED) +### Attempt: Move Interceptors to Instance Template -Workerd already migrated to `SetAccessorProperty` with `FunctionTemplate` for prototype properties in 2022: - -```cpp -auto getterFn = v8::FunctionTemplate::New(isolate, Gcb::callback); -prototype->SetAccessorProperty(v8Name, getterFn, ...); -``` - -This uses `FunctionCallbackInfo::This()` which is **NOT deprecated**. Workers with compatibility date >= 2022-01-31 use this pattern. - -### 2. Named Property Interceptors (CAN BE SOLVED) - -We discovered workerd puts interceptors on the **prototype**, while Chromium puts them on the **instance template**: +Chromium puts interceptors on `instance_template`, while workerd puts them on `prototype`: ```python # Chromium (interface.py line 5815) -interceptor_template = "${instance_template}" # Not prototype! +interceptor_template = "${instance_template}" ``` -When interceptor is on instance: `HolderV2() == This()` → can use `HolderV2()` -When interceptor is on prototype: `HolderV2() != This()` → need `This()` - -**Fix**: Change `registerWildcardProperty()` from: ```cpp -prototype->SetHandler(...) +// Workerd (resource.h) +prototype->SetHandler(...) // On prototype, not instance ``` -to: + +We tried changing workerd to match Chromium: + ```cpp -instance->SetHandler(...) +instance->SetHandler(...) // Changed to instance ``` -This matches Chromium and eliminates the need for `This()` in interceptors. See `interceptor-on-instance.patch`. +**Result**: JSG unit tests pass, but **RPC tests fail**: +``` +TypeError: stub.foo is not a function +TypeError: stub.increment is not a function +``` -### 3. Legacy Instance Properties (NEEDS SOLUTION) +### Why It Fails -Workers with compatibility date < 2022-01-31 use instance properties via `SetNativeDataProperty`. These need `This()` to detect "prototype-as-this" attacks: +Workerd uses prototype-level interceptors for **RPC stubs**. These are wrapper objects that intercept any property access and forward it over RPC: -```cpp -auto obj = info.HolderV2(); -if (obj != info.This()) { // Still need This() for this check - throwTypeError(isolate, kIllegalInvocation); -} +```javascript +const stub = env.MY_DURABLE_OBJECT.get(id); +stub.anyMethodName(args); // Intercepted and sent over RPC ``` -## Observable Differences +With interceptor on prototype: +- Works for any object inheriting from `JsRpcStub.prototype` +- Enables flexible RPC proxying -Moving interceptor from prototype to instance may affect: +With interceptor on instance: +- Only works for objects created directly from the template +- Breaks RPC stub pattern -| Operation | Prototype Interceptor | Instance Interceptor | -|-----------|----------------------|---------------------| -| `hasOwnProperty('x')` | Query callback NOT called | Query callback called (if defined) | -| `Object.keys(obj)` | Enumerator on prototype | Enumerator on instance | +This is fundamentally different from Chromium's use of interceptors (HTMLCollection, Storage, etc.) where the interceptor is on the actual object being accessed. -However, workerd uses `kNonMasking` and has no query callback, so impact may be minimal. +## What Workerd Needs -V8 test `InterceptorHasOwnProperty` (test-api-interceptors.cc:1071) confirms: -- With interceptor on instance: `hasOwnProperty` returns false for non-intercepted properties -- After setting the property: `hasOwnProperty` returns true +| Use Case | API Needed | Can Migrate? | +|----------|-----------|--------------| +| Prototype properties (new Workers) | `FunctionCallbackInfo::This()` | Already done (2022) | +| Named interceptors (RPC stubs) | `PropertyCallbackInfo::This()` | **No** - breaks RPC | +| Instance properties (legacy Workers) | `PropertyCallbackInfo::This()` | **No** - backwards compat | -## Questions for V8 Team +## Request for V8 Team -1. **For the legacy instance property case**: Is there a recommended alternative to `This()` for detecting when an object is accessed through the prototype chain vs directly? +`PropertyCallbackInfo::This()` is needed for workerd's legitimate use case of prototype-level interceptors. This is not a case of doing something wrong that can be fixed by following Chromium's pattern - we tested that and it breaks functionality. -2. **For interceptors**: V8 bug 455600234 says "For interceptors, using This() is always semantically correct." Does this mean `This()` will remain available for interceptors even after deprecation? +Options: +1. **Keep `This()` available** for prototype-level interceptors +2. **Provide alternative API** to get the receiver in `PropertyCallbackInfo` +3. **Clarify the migration path** for embedders who need prototype-level interceptors -3. **Timeline**: When will `This()` actually be removed vs just deprecated with warnings? +## Test Evidence -## Summary +```bash +# Without patch (interceptor on prototype): +bazel test '//src/workerd/api/tests:js-rpc-test@' # PASSED -| Use Case | Current Status | Solution | -|----------|---------------|----------| -| Prototype properties (new Workers) | Uses `FunctionCallbackInfo::This()` | Already solved | -| Named interceptors | Uses `PropertyCallbackInfo::This()` | Move to instance template | -| Instance properties (legacy Workers) | Uses `PropertyCallbackInfo::This()` | Needs `This()` or alternative | +# With patch (interceptor on instance): +bazel test '//src/workerd/api/tests:js-rpc-test@' # FAILED +# TypeError: stub.foo is not a function +# TypeError: stub.increment is not a function +``` --- From 0be8d8fad6b88fbcc2b6b56970f0dcc8a8a5249d Mon Sep 17 00:00:00 2001 From: Erik Corry Date: Thu, 22 Jan 2026 14:48:33 +0100 Subject: [PATCH 5/5] tone --- v8-this-deprecation-case.md | 39 ++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/v8-this-deprecation-case.md b/v8-this-deprecation-case.md index 1bab3d4e115..cd134a5a8a9 100644 --- a/v8-this-deprecation-case.md +++ b/v8-this-deprecation-case.md @@ -2,7 +2,7 @@ ## Summary -Cloudflare Workers (workerd) genuinely needs `PropertyCallbackInfo::This()` and **cannot follow Chromium's pattern**. We tested moving interceptors from prototype to instance template (like Chromium does) and it breaks RPC functionality. +Unfortunately, Cloudflare Workers (workerd) genuinely needs `PropertyCallbackInfo::This()` and cannot follow Chromium's pattern. We tested moving interceptors from prototype to instance template (like Chromium does) and it breaks RPC functionality. ## What We Tested @@ -26,7 +26,7 @@ We tried changing workerd to match Chromium: instance->SetHandler(...) // Changed to instance ``` -**Result**: JSG unit tests pass, but **RPC tests fail**: +Result: Our RPC tests fail: ``` TypeError: stub.foo is not a function TypeError: stub.increment is not a function @@ -34,7 +34,7 @@ TypeError: stub.increment is not a function ### Why It Fails -Workerd uses prototype-level interceptors for **RPC stubs**. These are wrapper objects that intercept any property access and forward it over RPC: +Workerd uses prototype-level interceptors for RPC stubs. These are wrapper objects that intercept any property access and forward it over RPC: ```javascript const stub = env.MY_DURABLE_OBJECT.get(id); @@ -51,6 +51,17 @@ With interceptor on instance: This is fundamentally different from Chromium's use of interceptors (HTMLCollection, Storage, etc.) where the interceptor is on the actual object being accessed. +## Backwards compatibility issue + +In addition to the RPC issue we also have a backwards compatibility issue. Workers take +backwards compatibility very seriously - we can't break already-deployed scripts when +we run them with a newer version of V8. + +In 2022 we moved to setting accessors on the prototype instead of on the instance. +This solved some compatibility issues, eg. people could not subclass and get the +expected behaviour. It also aligns better with IDL. However we still have a +compatibility flag for this since it is trivially observable from JS. + ## What Workerd Needs | Use Case | API Needed | Can Migrate? | @@ -59,26 +70,14 @@ This is fundamentally different from Chromium's use of interceptors (HTMLCollect | Named interceptors (RPC stubs) | `PropertyCallbackInfo::This()` | **No** - breaks RPC | | Instance properties (legacy Workers) | `PropertyCallbackInfo::This()` | **No** - backwards compat | -## Request for V8 Team +## Conclusion -`PropertyCallbackInfo::This()` is needed for workerd's legitimate use case of prototype-level interceptors. This is not a case of doing something wrong that can be fixed by following Chromium's pattern - we tested that and it breaks functionality. +`PropertyCallbackInfo::This()` is needed for workerd's legitimate use case of prototype-level interceptors. This is not a case of something that can be fixed by following Chromium's pattern - we tested that and it breaks functionality. Options: -1. **Keep `This()` available** for prototype-level interceptors -2. **Provide alternative API** to get the receiver in `PropertyCallbackInfo` -3. **Clarify the migration path** for embedders who need prototype-level interceptors - -## Test Evidence - -```bash -# Without patch (interceptor on prototype): -bazel test '//src/workerd/api/tests:js-rpc-test@' # PASSED - -# With patch (interceptor on instance): -bazel test '//src/workerd/api/tests:js-rpc-test@' # FAILED -# TypeError: stub.foo is not a function -# TypeError: stub.increment is not a function -``` +1. Keep `This()` available for prototype-level interceptors +2. Provide alternative API to get the receiver in `PropertyCallbackInfo` +3. Clarify the migration path for embedders who need prototype-level interceptors ---