dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.21k stars 1.57k forks source link

[dart2js] Runtime error calling an interop callback with more arguments than parameters #54342

Open annagrin opened 10 months ago

annagrin commented 10 months ago

The following code causes a runtime error, not sure what the proper fix would be:

Code

import 'dart:js_interop';
import 'package:web/helpers.dart';

import 'web_utils.dart';

// According to the CSP3 spec a nonce must be a valid base64 string.
final _noncePattern = RegExp('^[\\w+/_-]+[=]{0,2}\$');

/// Returns CSP nonce, if set for any script tag.
String? _findNonce() {
  final elements = window.document.querySelectorAll('script');
  elements.forEach(
    (Node element) {
      final nonceValue = (element as HtmlElement).nonce;
      if (_noncePattern.hasMatch(nonceValue)) {
        return nonceValue;
      }
    }.toJS,
  );
  return null;
}

/// Creates a script that will run properly when strict CSP is enforced.
///
/// More specifically, the script has the correct `nonce` value set.
HTMLScriptElement _createScript() {
  final nonce = _findNonce();

  return nonce == null ? HTMLScriptElement() : HTMLScriptElement()
    ..setAttribute('nonce', nonce!);
}

/// Runs `window.$dartRunMain()` by injecting a script tag.
///
/// We do this so that we don't see user exceptions bubble up in our own error
/// handling zone.
void runMain() {
  final scriptElement = _createScript()..htmlFor = r'window.$dartRunMain();';
  (document.body as HTMLBodyElement).append(scriptElement.toJSBox);
  // External tear-offs are not allowed.
  // ignore: unnecessary_lambdas
  Future.microtask(() => scriptElement.remove());
}

Runtime error

NoSuchMethodError: method not found: 'call'
Receiver: Closure '_findNonce_closure'
Arguments: [Instance of 'HtmlElement', 0, Instance of 'NodeList']
NoSuchMethodError: method not found: 'call'
Receiver: Closure '_findNonce_closure'
Arguments: [Instance of 'HtmlElement', 0, Instance of 'NodeList']
    at Object.wrapException (http://127.0.0.1:56644/dwds/src/injected/client.js:1347:43)
    at _findNonce_closure.noSuchMethod$1 (http://127.0.0.1:56644/dwds/src/injected/client.js:16081:15)
    at Object.noSuchMethod$1$ (http://127.0.0.1:56644/dwds/src/injected/client.js:543:42)
    at Object.Primitives_functionNoSuchMethod (http://127.0.0.1:56644/dwds/src/injected/client.js:1212:16)
    at Object.Primitives__generalApplyFunction (http://127.0.0.1:56644/dwds/src/injected/client.js:1268:18)
    at Object.Primitives_applyFunction (http://127.0.0.1:56644/dwds/src/injected/client.js:1244:16)
    at Object.Function_apply (http://127.0.0.1:56644/dwds/src/injected/client.js:5812:16)
    at _callDartFunctionFast (http://127.0.0.1:56644/dwds/src/injected/client.js:7962:16)
    at http://127.0.0.1:56644/dwds/src/injected/client.js:7953:18
    at NodeList.forEach (<anonymous>)
    at Object._findNonce (http://127.0.0.1:56644/dwds/src/injected/client.js:9479:79)
    at Object.runMain (http://127.0.0.1:56644/dwds/src/injected/client.js:9484:19)
    at http://127.0.0.1:56644/dwds/src/injected/client.js:24311:19
    at _wrapJsFunctionForAsync_closure.$protected (http://127.0.0.1:56644/dwds/src/injected/client.js:3906:15)
    at _wrapJsFunctionForAsync_closure.call$2 (http://127.0.0.1:56644/dwds/src/injected/client.js:11730:12)
    at Object._asyncStartSync (http://127.0.0.1:56644/dwds/src/injected/client.js:3870:20)
    at main__closure4.$call$body$main__closure (http://127.0.0.1:56644/dwds/src/injected/client.js:24320:16)
    at main__closure4.call$1 (http://127.0.0.1:56644/dwds/src/injected/client.js:24247:19)
    at StaticClosure._rootRunUnary (http://127.0.0.1:56644/dwds/src/injected/client.js:4289:16)
    at _CustomZone.runUnary$2$2 (http://127.0.0.1:56644/dwds/src/injected/client.js:13125:39)
    at _CustomZone.runUnaryGuarded$1$2 (http://127.0.0.1:56644/dwds/src/injected/client.js:13072:14)
    at _ControllerSubscription._sendData$1 (http://127.0.0.1:56644/dwds/src/injected/client.js:12665:19)
    at _DelayedData.perform$1 (http://127.0.0.1:56644/dwds/src/injected/client.js:12848:59)
    at _PendingEvents_schedule_closure.call$0 (http://127.0.0.1:56644/dwds/src/injected/client.js:12907:14)
    at Object._microtaskLoop (http://127.0.0.1:56644/dwds/src/injected/client.js:4129:24)
    at StaticClosure._startMicrotaskLoop (http://127.0.0.1:56644/dwds/src/injected/client.js:4135:11)
    at _AsyncRun__initializeScheduleImmediate_internalCallback.call$1 (http://127.0.0.1:56644/dwds/src/injected/client.js:11607:9)

Repro

  1. clone the following PR: https://github.com/dart-lang/webdev/pull/2305
  2. run dart run build_runner build in webdev/dwds directory
  3. debug webdev/fixtures/_webdevSoundSmoke example in VSCode
  4. open the chrome debugger when chrome starts, see the trace above in the console tab
kevmoo commented 10 months ago

CC @srujzs

annagrin commented 10 months ago

Btw looks like there is a naming problem (call$1 vs call)- here is prototype generated by dart2js for the closure in question:

A._findNonce_closure.prototype = {
    call$1(element) {
      var nonceValue = A._asString(type$.JavaScriptObject._as(element).nonce),
        t1 = $.$get$_noncePattern();
      if (t1._nativeRegExp.test(nonceValue))
        this._box_0.nonce = nonceValue;
    },
    $signature: 77
  };
sigmundch commented 10 months ago

I haven't debugged the code yet, but I'm debating two theories:

Looking at the output of client.js, I believe (b) is more likely than (a). First, I see the code that calls .forEach, and it appears to be the raw JSInterop invocation from the extension method and not an intercepted call from dart:html (in that case I'd expect the call to be .forEach$1). The fact that the NoSuchMethod error also shows 3 arguments above also leads me to believe this could be (b).

Would it work if you change the code above to not just expect Node, but Node, int, NodeList (matching what MDN describes here https://developer.mozilla.org/en-US/docs/Web/API/NodeList/forEach)? The second and third argument could be optional too: (Node element, [int index, NodeList list]) { ... }.toJS.

sigmundch commented 10 months ago

I haven't tried the original code in dwds, but I can confirm that a simple example with mismatched arguments shows a similar error.

Here is an isolated repro:

import 'dart:js_interop';
import 'dart:js_interop_unsafe';

extension type F(JSObject o) implements JSObject {
  external void forEach(JSFunction f);
}

@JS()
external F get f;

main() {
  globalContext.callMethod('eval'.toJS,
      'window.f = {forEach(g) { g(1,2); },};'.toJS);
  f.forEach(((JSAny a, JSAny b) => print('$a $b')).toJS); // good
  f.forEach(((JSAny a) => print('$a')).toJS);             // oh no!
}

This produces this error when running in d8:

1 2
/usr/local/google/home/sigmund/dart/git_all/sdk/sdk/lib/_internal/js_runtime/lib/preambles/d8.js:263: NoSuchMethodError: method not found: 'call'
Receiver: Closure 'main_closure0'
Arguments: [1, 2]
          throw e;
          ^
NoSuchMethodError: method not found: 'call'
Receiver: Closure 'main_closure0'
Arguments: [1, 2]
    at Object.wrapException (/tmp/out.js:669:43)
    at main_closure0.noSuchMethod$1 (/tmp/out.js:3863:15)
    at Object.noSuchMethod$1$ (/tmp/out.js:398:42)
    at Object.Primitives_functionNoSuchMethod (/tmp/out.js:554:16)
    at Object.Primitives__generalApplyFunction (/tmp/out.js:607:18)
    at Object.Primitives_applyFunction (/tmp/out.js:586:16)
    at _callDartFunctionFast (/tmp/out.js:2978:16)
    at /tmp/out.js:2968:18
    at Object.forEach (eval at JSObjectUnsafeUtilExtension__callMethod (/tmp/out.js:2958:29), <anonymous>:1:26)
    at Object.callMethod (/tmp/out.js:2987:31)

Finished with a exit code: 1

It has often been requested that we are more permissive with extra arguments on callbacks, so it may be worth revisiting that in the near future

annagrin commented 10 months ago

the workaround for me was to not use the "foreach" and just iterate with an index:) Leaving this as a bug though because it would be great if this example did not compile.

srujzs commented 10 months ago

I think Siggi nailed it that there's a mismatch in the number of parameters of the callback and the number of arguments JS provides. Here's the request for wrapped Dart functions to accept more parameters: https://github.com/dart-lang/sdk/issues/48186

the workaround for me was to not use the "foreach" and just iterate with an index:) Leaving this as a bug though because it would be great if this example did not compile.

I think this is hard to do given the current way we write JS types. We don't know how many args a given callback is expected to take with JSFunction. There is a general feature request I've been meaning to file and thinking about which is making JSFunction a generic type that accepts a function type.

In this case, it would look something like JSFunction<void Function(Node, int, NodeList)>, so passing any arbitrary JSFunction won't work. This would make the above code not compile (but of course, if you casted, it'll compile).

joranmulderij commented 10 months ago

This is a little example that might be useful.

class CallableObject {
  const CallableObject();

  void call() {
    print("CallableObject called");
  }
}

class ValueContainer<T> {
  ValueContainer(this.value);
  final T value;
  final value2 = const CallableObject();
}

void main() {
  final b = ValueContainer(const CallableObject());
  b.value2(); // Runs correctly and calls the "call" function
  b.value.call(); // Also runs correctly
  b.value(); // NoSuchMethodError: tried to call a non-function, such as null: 'b.value'
}
srujzs commented 10 months ago

I think that's a tangential issue as it doesn't use callbacks or interop. I've filed a separate issue here: https://github.com/dart-lang/sdk/issues/54461