Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent presence of property in contextified object #52720

Open
lachrist opened this issue Apr 27, 2024 · 3 comments
Open

Inconsistent presence of property in contextified object #52720

lachrist opened this issue Apr 27, 2024 · 3 comments
Labels
vm Issues and PRs related to the vm subsystem.

Comments

@lachrist
Copy link

Version

v22.0.0

Platform

Darwin Laurents-MacBook-Air.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:59:33 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T8112 arm64

Subsystem

vm

What steps will reproduce the bug?

import { createContext, runInContext } from "vm";
console.log(
  runInContext(
    `
      "use strict";
      ({
        hasOwn: Object.hasOwn(globalThis, "toLocaleString"),
        descriptor: Reflect.getOwnPropertyDescriptor(globalThis, "toLocaleString"),
      });
    `,
    createContext(),
  ),
); // { hasOwn: true, descriptor: undefined }

Tested in both v22.0.0 and v20.12.2.

How often does it reproduce? Is there a required condition?

No required condition.

What is the expected behavior? Why is that the expected behavior?

Either the property is present or it is not. So either { hasOwn: false, descriptor: undefined} and { hasOwn: true, descriptor: { ... } } would be better.

What do you see instead?

{ hasOwn: true, descriptor: undefined }

Additional information

No response

@benjamingr
Copy link
Member

@nodejs/vm

@targos
Copy link
Member

targos commented Apr 30, 2024

I understand why it happens:

  • Object.has calls our PropertyGetterCallback interceptor
  • We call GetRealNamedProperty, which follows the prototype chain

node/src/node_contextify.cc

Lines 477 to 478 in 6aa9047

maybe_rv =
ctx->global_proxy()->GetRealNamedProperty(context, property);

I don't know how to fix it though. There are cases where we want to follow the protoype and other where we don't, but I'm not sure V8 gives us enough information to know in which case we are.

@targos
Copy link
Member

targos commented Apr 30, 2024

Maybe the solution is to set a NamedPropertyQueryCallback (we currently don't).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants