1742871 - Cover the Change Array by copy proposal methods in Xray

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

We handle built-in functions/properties specially in Xray,
so privileged script can call content script's function safely without getting affected by modified prototype/constructor methods.

they're handled in the following 2 places:

https://searchfox.org/mozilla-central/rev/553bf8428885dbd1eab9b63f71ef647f799372c2/js/xpconnect/wrappers/XrayWrapper.cpp#720-722

bool JSXrayTraits::resolveOwnProperty(
    JSContext* cx, HandleObject wrapper, HandleObject target,
    HandleObject holder, HandleId id,
    MutableHandle<Maybe<PropertyDescriptor>> desc) {
...
  // Grab the JSClass. We require all Xrayable classes to have a ClassSpec.
  const JSClass* clasp = JS::GetClass(target);
  MOZ_ASSERT(clasp->specDefined());

  // Indexed array properties are handled above, so we can just work with the
  // class spec here.
  return TryResolvePropertyFromSpecs(cx, id, holder,
                                     clasp->specPrototypeFunctions(),
                                     clasp->specPrototypeProperties(), desc);
}

https://searchfox.org/mozilla-central/rev/553bf8428885dbd1eab9b63f71ef647f799372c2/js/xpconnect/wrappers/XrayWrapper.cpp#1006-1008

bool JSXrayTraits::enumerateNames(JSContext* cx, HandleObject wrapper,
                                  unsigned flags, MutableHandleIdVector props) {
...
  // Grab the JSClass. We require all Xrayable classes to have a ClassSpec.
  const JSClass* clasp = JS::GetClass(target);
  MOZ_ASSERT(clasp->specDefined());

  return AppendNamesFromFunctionAndPropertySpecs(
      cx, key, clasp->specPrototypeFunctions(),
      clasp->specPrototypeProperties(), flags, props);
}

clasp->specPrototypeFunctions() returns js::ClassSpec::prototypeFunctions:

https://searchfox.org/mozilla-central/rev/553bf8428885dbd1eab9b63f71ef647f799372c2/js/public/Class.h#419

struct MOZ_STATIC_CLASS ClassSpec {
  ClassObjectCreationOp createConstructor;
  ClassObjectCreationOp createPrototype;
  const JSFunctionSpec* constructorFunctions;
  const JSPropertySpec* constructorProperties;
  const JSFunctionSpec* prototypeFunctions;
  const JSPropertySpec* prototypeProperties;
  FinishClassInitOp finishInit;
...

So, functions listed there are automatically handled there, but if we define more prototype functions in finishInit, they're not automatically covered by Xray.

The Change Array by copy methods matches this case.
those new methods are listed in separate change_array_by_copy_methods array,
and defined on the prototype object in finishInit.

https://searchfox.org/mozilla-central/rev/553bf8428885dbd1eab9b63f71ef647f799372c2/js/src/builtin/Array.cpp#3868-3876

#ifdef ENABLE_CHANGE_ARRAY_BY_COPY
static const JSFunctionSpec change_array_by_copy_methods[] = {
    JS_SELF_HOSTED_FN("withReversed", "ArrayWithReversed", 0, 0),
    JS_SELF_HOSTED_FN("withSorted", "ArrayWithSorted", 1, 0),
    JS_FN("withSpliced", array_with_spliced, 2, 0),
    JS_FN("withAt", array_with_at, 2, 0),

    JS_FS_END};
#endif

https://searchfox.org/mozilla-central/rev/553bf8428885dbd1eab9b63f71ef647f799372c2/js/src/builtin/Array.cpp#4127-4140
#ifdef ENABLE_CHANGE_ARRAY_BY_COPY

static bool array_proto_finish(JSContext* cx, JS::HandleObject ctor,
                               JS::HandleObject proto) {
...
  if (cx->options().changeArrayByCopy()) {
...
    if (!JS_DefineFunctions(cx, proto, change_array_by_copy_methods)) {
      return false;
    }
  }
#endif
...
}

To support them in Xray, they needs to be handled in JSXrayTraits::resolveOwnProperty and JSXrayTraits::enumerateNames.

Comment 1

[Tooru Fujisawa [:arai]]

Given it's still stage 2 and behind build configuration, this isn't urgent,
also, some other proposals will need the same, so I've filed bug 1742876 to cover all of them.
this can be fixed after the bug 1742876.

Comment 2

[Tooru Fujisawa [:arai]]

This can be an example usage for bug 1742876 patch, so I'll take this as well.

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1742871 - Support Change Array by copy proposal methods in Xray. r?jandem!

Depends on D132105

Comment 4

[Tooru Fujisawa [:arai]]

as pointed out by jandem, this should be fixed by moving the change_array_by_copy_methods entries into array_methods, and list them up in js::ShouldIgnorePropertyDefinition, instead of modifying Xray,

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1742871 - Move Change Array by copy methods into ClassSpec to support them in Xray. r?jandem!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6ced3ae7460
Move Change Array by copy methods into ClassSpec to support them in Xray. r=jandem

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/b6ced3ae7460