From f3293dbe0fbd3221299dc3207036dcc5223c8e18 Mon Sep 17 00:00:00 2001 From: Goodwine <2022649+Goodwine@users.noreply.github.com> Date: Thu, 3 Nov 2022 16:50:06 -0700 Subject: [PATCH] JS API: Validate that importer result 'contents' is a `string` and improve ArgumentError output (#1816) * Validate ImporterResult 'contents' and improve ArgumentError output * only use JS stuff in the nodejs bindings * handle non-string contents for legacy importer too * make it work with node 12 --- CHANGELOG.md | 6 +++++ .../importer/legacy_node/implementation.dart | 5 ++++ lib/src/importer/node_to_dart/async.dart | 5 ++++ lib/src/importer/node_to_dart/sync.dart | 5 ++++ lib/src/node/function.dart | 25 +++++++++++++++++++ lib/src/node/utils.dart | 19 ++++++++++++++ lib/src/visitor/async_evaluate.dart | 2 ++ lib/src/visitor/evaluate.dart | 4 ++- 8 files changed, 70 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95d87efde..4813a7e83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,6 +60,12 @@ * Emit a deprecation warning when passing a `sassIndex` with units to `Value.sassIndexToListIndex()`. This will be an error in Dart Sass 2.0.0. +### JS API + +* Importer results now validate whether `contents` is actually a string type. + +* Importer result argument errors are now rendered correctly. + ## 1.55.0 * **Potentially breaking bug fix:** Sass numbers are now universally stored as diff --git a/lib/src/importer/legacy_node/implementation.dart b/lib/src/importer/legacy_node/implementation.dart index 1d7ffcd78..c08d5a30e 100644 --- a/lib/src/importer/legacy_node/implementation.dart +++ b/lib/src/importer/legacy_node/implementation.dart @@ -183,6 +183,11 @@ class NodeImporter { var file = value.file; var contents = value.contents; + if (contents != null && !isJsString(contents)) { + jsThrow(ArgumentError.value(contents, 'contents', + 'must be a string but was: ${jsType(contents)}')); + } + if (file == null) { return Tuple2(contents ?? '', url); } else if (contents != null) { diff --git a/lib/src/importer/node_to_dart/async.dart b/lib/src/importer/node_to_dart/async.dart index bc900c8e6..5cf06e225 100644 --- a/lib/src/importer/node_to_dart/async.dart +++ b/lib/src/importer/node_to_dart/async.dart @@ -43,6 +43,11 @@ class NodeToDartAsyncImporter extends AsyncImporter { result as NodeImporterResult; var contents = result.contents; + if (!isJsString(contents)) { + jsThrow(ArgumentError.value(contents, 'contents', + 'must be a string but was: ${jsType(contents)}')); + } + var syntax = result.syntax; if (contents == null || syntax == null) { jsThrow(JsError("The load() function must return an object with contents " diff --git a/lib/src/importer/node_to_dart/sync.dart b/lib/src/importer/node_to_dart/sync.dart index 87f145b40..4352d8d3e 100644 --- a/lib/src/importer/node_to_dart/sync.dart +++ b/lib/src/importer/node_to_dart/sync.dart @@ -48,6 +48,11 @@ class NodeToDartImporter extends Importer { result as NodeImporterResult; var contents = result.contents; + if (!isJsString(contents)) { + jsThrow(ArgumentError.value(contents, 'contents', + 'must be a string but was: ${jsType(contents)}')); + } + var syntax = result.syntax; if (contents == null || syntax == null) { jsThrow(JsError("The load() function must return an object with contents " diff --git a/lib/src/node/function.dart b/lib/src/node/function.dart index 70869827a..18ec907f1 100644 --- a/lib/src/node/function.dart +++ b/lib/src/node/function.dart @@ -6,6 +6,31 @@ import 'package:js/js.dart'; @JS("Function") class JSFunction { + /// Creates a [JS function]. + /// + /// The **last** argument is the function body. The other arguments become the + /// function's parameters. + /// + /// The function body must declare a `return` statement in order to return a + /// value, otherwise it returns [JSNull]. + /// + /// Note: The function body must be compatible with Node 12. Null coalescing + /// and optional chaining features are not supported. + /// + /// Examples: + /// ```dart + /// var sum = JSFunction('a', 'b', 'return a + b'); + /// sum.call(13, 29) as int; // 42 + /// + /// var isJsString = JSFunction('a', 'return typeof a === "string"'); + /// isJsString.call(42) as bool; // false + /// isJsString.call('42') as bool; // true + /// + /// var sayHi = JSFunction('console.log("Hi!")'); + /// sayHi.call(); // Logs "Hi!" + /// ``` + /// + /// [JS Function]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/Function#syntax external JSFunction(String arg1, [String? arg2, String? arg3]); // Note that this just invokes the function with the given arguments, rather diff --git a/lib/src/node/utils.dart b/lib/src/node/utils.dart index 235e59749..168993098 100644 --- a/lib/src/node/utils.dart +++ b/lib/src/node/utils.dart @@ -72,6 +72,25 @@ void jsForEach(Object object, void callback(String key, Object? value)) { /// returns `null`. Object? jsEval(String js) => JSFunction('', js).call(); +/// Returns whether the [object] is a JS `string`. +bool isJsString(Object? object) => _jsTypeOf(object) == 'string'; + +/// Returns the [object]'s `typeof` according to the JS engine. +String _jsTypeOf(Object? object) => + JSFunction("value", "return typeof value").call(object) as String; + +/// Returns `typeof value` if [value] is a native type, otherwise returns the +/// [value]'s JS class name. +String jsType(Object? value) { + var typeOf = _jsTypeOf(value); + return typeOf != 'object' ? typeOf : JSFunction('value', ''' + if (value && value.constructor && value.constructor.name) { + return value.constructor.name; + } + return "object"; + ''').call(value) as String; +} + @JS("Object.defineProperty") external void _defineProperty( Object object, String name, _PropertyDescriptor prototype); diff --git a/lib/src/visitor/async_evaluate.dart b/lib/src/visitor/async_evaluate.dart index cb0c9f6d3..39b935463 100644 --- a/lib/src/visitor/async_evaluate.dart +++ b/lib/src/visitor/async_evaluate.dart @@ -1645,6 +1645,8 @@ class _EvaluateVisitor } } on SassException catch (error, stackTrace) { throwWithTrace(_exception(error.message, error.span), stackTrace); + } on ArgumentError catch (error, stackTrace) { + throwWithTrace(_exception(error.toString()), stackTrace); } catch (error, stackTrace) { String? message; try { diff --git a/lib/src/visitor/evaluate.dart b/lib/src/visitor/evaluate.dart index 22228e44e..e45ad4b97 100644 --- a/lib/src/visitor/evaluate.dart +++ b/lib/src/visitor/evaluate.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_evaluate.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: c70a4193cc291f298f601a5cc371be9eac71fb74 +// Checksum: a14e075a5435c7457d1d1371d8b97dd327a66ec4 // // ignore_for_file: unused_import @@ -1643,6 +1643,8 @@ class _EvaluateVisitor } } on SassException catch (error, stackTrace) { throwWithTrace(_exception(error.message, error.span), stackTrace); + } on ArgumentError catch (error, stackTrace) { + throwWithTrace(_exception(error.toString()), stackTrace); } catch (error, stackTrace) { String? message; try {