diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f8fdc4f2..ad796a58c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,6 +137,7 @@ jobs: sass_spec_js_embedded: name: 'JS API Tests | Embedded | Node ${{ matrix.node-version }} | ${{ matrix.os }}' runs-on: ${{ matrix.os }}-latest + if: "github.event_name != 'pull_request' || !contains(github.event.pull_request.body, 'skip sass-embedded')" strategy: fail-fast: false diff --git a/lib/src/async_import_cache.dart b/lib/src/async_import_cache.dart index 019b3414a..33fc26cce 100644 --- a/lib/src/async_import_cache.dart +++ b/lib/src/async_import_cache.dart @@ -122,6 +122,9 @@ final class AsyncImportCache { /// Canonicalizes [url] according to one of this cache's importers. /// + /// The [baseUrl] should be the canonical URL of the stylesheet that contains + /// the load, if it exists. + /// /// Returns the importer that was used to canonicalize [url], the canonical /// URL, and the URL that was passed to the importer (which may be resolved /// relative to [baseUrl] if it's passed). @@ -139,33 +142,30 @@ final class AsyncImportCache { if (isBrowser && (baseImporter == null || baseImporter is NoOpImporter) && _importers.isEmpty) { - throw "Custom importers are required to load stylesheets when compiling in the browser."; + throw "Custom importers are required to load stylesheets when compiling " + "in the browser."; } if (baseImporter != null && url.scheme == '') { - var relativeResult = await putIfAbsentAsync(_relativeCanonicalizeCache, ( - url, - forImport: forImport, - baseImporter: baseImporter, - baseUrl: baseUrl - ), () async { - var resolvedUrl = baseUrl?.resolveUri(url) ?? url; - if (await _canonicalize(baseImporter, resolvedUrl, forImport) - case var canonicalUrl?) { - return (baseImporter, canonicalUrl, originalUrl: resolvedUrl); - } else { - return null; - } - }); + var relativeResult = await putIfAbsentAsync( + _relativeCanonicalizeCache, + ( + url, + forImport: forImport, + baseImporter: baseImporter, + baseUrl: baseUrl + ), + () => _canonicalize(baseImporter, baseUrl?.resolveUri(url) ?? url, + baseUrl, forImport)); if (relativeResult != null) return relativeResult; } return await putIfAbsentAsync( _canonicalizeCache, (url, forImport: forImport), () async { for (var importer in _importers) { - if (await _canonicalize(importer, url, forImport) - case var canonicalUrl?) { - return (importer, canonicalUrl, originalUrl: url); + if (await _canonicalize(importer, url, baseUrl, forImport) + case var result?) { + return result; } } @@ -175,18 +175,36 @@ final class AsyncImportCache { /// Calls [importer.canonicalize] and prints a deprecation warning if it /// returns a relative URL. - Future _canonicalize( - AsyncImporter importer, Uri url, bool forImport) async { - var result = await (forImport - ? inImportRule(() => importer.canonicalize(url)) - : importer.canonicalize(url)); - if (result?.scheme == '') { - _logger.warnForDeprecation(Deprecation.relativeCanonical, """ -Importer $importer canonicalized $url to $result. -Relative canonical URLs are deprecated and will eventually be disallowed. -"""); + /// + /// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl] + /// before passing it to [importer]. + Future _canonicalize( + AsyncImporter importer, Uri url, Uri? baseUrl, bool forImport, + {bool resolveUrl = false}) async { + var resolved = + resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url; + var canonicalize = forImport + ? () => inImportRule(() => importer.canonicalize(resolved)) + : () => importer.canonicalize(resolved); + + var passContainingUrl = baseUrl != null && + (url.scheme == '' || await importer.isNonCanonicalScheme(url.scheme)); + var result = await withContainingUrl( + passContainingUrl ? baseUrl : null, canonicalize); + if (result == null) return null; + + if (result.scheme == '') { + _logger.warnForDeprecation( + Deprecation.relativeCanonical, + "Importer $importer canonicalized $resolved to $result.\n" + "Relative canonical URLs are deprecated and will eventually be " + "disallowed."); + } else if (await importer.isNonCanonicalScheme(result.scheme)) { + throw "Importer $importer canonicalized $resolved to $result, which " + "uses a scheme declared as non-canonical."; } - return result; + + return (importer, result, originalUrl: resolved); } /// Tries to import [url] using one of this cache's importers. diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart index 22df57fca..d99098735 100644 --- a/lib/src/embedded/dispatcher.dart +++ b/lib/src/embedded/dispatcher.dart @@ -206,19 +206,32 @@ final class Dispatcher { InboundMessage_CompileRequest_Importer importer) { switch (importer.whichImporter()) { case InboundMessage_CompileRequest_Importer_Importer.path: + _checkNoNonCanonicalScheme(importer); return sass.FilesystemImporter(importer.path); case InboundMessage_CompileRequest_Importer_Importer.importerId: - return HostImporter(this, importer.importerId); + return HostImporter( + this, importer.importerId, importer.nonCanonicalScheme); case InboundMessage_CompileRequest_Importer_Importer.fileImporterId: + _checkNoNonCanonicalScheme(importer); return FileImporter(this, importer.fileImporterId); case InboundMessage_CompileRequest_Importer_Importer.notSet: + _checkNoNonCanonicalScheme(importer); return null; } } + /// Throws a [ProtocolError] if [importer] contains one or more + /// `nonCanonicalScheme`s. + void _checkNoNonCanonicalScheme( + InboundMessage_CompileRequest_Importer importer) { + if (importer.nonCanonicalScheme.isEmpty) return; + throw paramsError("Importer.non_canonical_scheme may only be set along " + "with Importer.importer.importer_id"); + } + /// Sends [event] to the host. void sendLog(OutboundMessage_LogEvent event) => _send(OutboundMessage()..logEvent = event); @@ -278,9 +291,7 @@ final class Dispatcher { InboundMessage_Message.versionRequest => throw paramsError("VersionRequest must have compilation ID 0."), InboundMessage_Message.notSet => - throw parseError("InboundMessage.message is not set."), - _ => - throw parseError("Unknown message type: ${message.toDebugString()}") + throw parseError("InboundMessage.message is not set.") }; if (message.id != _outboundRequestId) { diff --git a/lib/src/embedded/importer/file.dart b/lib/src/embedded/importer/file.dart index 8250515eb..0dbccfed5 100644 --- a/lib/src/embedded/importer/file.dart +++ b/lib/src/embedded/importer/file.dart @@ -24,11 +24,14 @@ final class FileImporter extends ImporterBase { Uri? canonicalize(Uri url) { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); - var response = - dispatcher.sendFileImportRequest(OutboundMessage_FileImportRequest() - ..importerId = _importerId - ..url = url.toString() - ..fromImport = fromImport); + var request = OutboundMessage_FileImportRequest() + ..importerId = _importerId + ..url = url.toString() + ..fromImport = fromImport; + if (containingUrl case var containingUrl?) { + request.containingUrl = containingUrl.toString(); + } + var response = dispatcher.sendFileImportRequest(request); switch (response.whichResult()) { case InboundMessage_FileImportResponse_Result.fileUrl: @@ -49,5 +52,7 @@ final class FileImporter extends ImporterBase { ImporterResult? load(Uri url) => _filesystemImporter.load(url); + bool isNonCanonicalScheme(String scheme) => scheme != 'file'; + String toString() => "FileImporter"; } diff --git a/lib/src/embedded/importer/host.dart b/lib/src/embedded/importer/host.dart index a1d5eed31..5819be1ec 100644 --- a/lib/src/embedded/importer/host.dart +++ b/lib/src/embedded/importer/host.dart @@ -2,7 +2,10 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import '../../exception.dart'; import '../../importer.dart'; +import '../../importer/utils.dart'; +import '../../util/span.dart'; import '../dispatcher.dart'; import '../embedded_sass.pb.dart' hide SourceSpan; import '../utils.dart'; @@ -13,14 +16,31 @@ final class HostImporter extends ImporterBase { /// The host-provided ID of the importer to invoke. final int _importerId; - HostImporter(Dispatcher dispatcher, this._importerId) : super(dispatcher); + /// The set of URL schemes that this importer promises never to return from + /// [canonicalize]. + final Set _nonCanonicalSchemes; + + HostImporter(Dispatcher dispatcher, this._importerId, + Iterable nonCanonicalSchemes) + : _nonCanonicalSchemes = Set.unmodifiable(nonCanonicalSchemes), + super(dispatcher) { + for (var scheme in _nonCanonicalSchemes) { + if (isValidUrlScheme(scheme)) continue; + throw SassException( + '"$scheme" isn\'t a valid URL scheme (for example "file").', + bogusSpan); + } + } Uri? canonicalize(Uri url) { - var response = - dispatcher.sendCanonicalizeRequest(OutboundMessage_CanonicalizeRequest() - ..importerId = _importerId - ..url = url.toString() - ..fromImport = fromImport); + var request = OutboundMessage_CanonicalizeRequest() + ..importerId = _importerId + ..url = url.toString() + ..fromImport = fromImport; + if (containingUrl case var containingUrl?) { + request.containingUrl = containingUrl.toString(); + } + var response = dispatcher.sendCanonicalizeRequest(request); return switch (response.whichResult()) { InboundMessage_CanonicalizeResponse_Result.url => @@ -47,5 +67,8 @@ final class HostImporter extends ImporterBase { }; } + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); + String toString() => "HostImporter"; } diff --git a/lib/src/import_cache.dart b/lib/src/import_cache.dart index b9c48a6f8..bc526d7a3 100644 --- a/lib/src/import_cache.dart +++ b/lib/src/import_cache.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_import_cache.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: 3e4cae79c03ce2af6626b1822f1468523b401e90 +// Checksum: ff52307a3bc93358ddc46f1e76120894fa3e071f // // ignore_for_file: unused_import @@ -124,6 +124,9 @@ final class ImportCache { /// Canonicalizes [url] according to one of this cache's importers. /// + /// The [baseUrl] should be the canonical URL of the stylesheet that contains + /// the load, if it exists. + /// /// Returns the importer that was used to canonicalize [url], the canonical /// URL, and the URL that was passed to the importer (which may be resolved /// relative to [baseUrl] if it's passed). @@ -139,31 +142,27 @@ final class ImportCache { if (isBrowser && (baseImporter == null || baseImporter is NoOpImporter) && _importers.isEmpty) { - throw "Custom importers are required to load stylesheets when compiling in the browser."; + throw "Custom importers are required to load stylesheets when compiling " + "in the browser."; } if (baseImporter != null && url.scheme == '') { - var relativeResult = _relativeCanonicalizeCache.putIfAbsent(( - url, - forImport: forImport, - baseImporter: baseImporter, - baseUrl: baseUrl - ), () { - var resolvedUrl = baseUrl?.resolveUri(url) ?? url; - if (_canonicalize(baseImporter, resolvedUrl, forImport) - case var canonicalUrl?) { - return (baseImporter, canonicalUrl, originalUrl: resolvedUrl); - } else { - return null; - } - }); + var relativeResult = _relativeCanonicalizeCache.putIfAbsent( + ( + url, + forImport: forImport, + baseImporter: baseImporter, + baseUrl: baseUrl + ), + () => _canonicalize(baseImporter, baseUrl?.resolveUri(url) ?? url, + baseUrl, forImport)); if (relativeResult != null) return relativeResult; } return _canonicalizeCache.putIfAbsent((url, forImport: forImport), () { for (var importer in _importers) { - if (_canonicalize(importer, url, forImport) case var canonicalUrl?) { - return (importer, canonicalUrl, originalUrl: url); + if (_canonicalize(importer, url, baseUrl, forImport) case var result?) { + return result; } } @@ -173,17 +172,36 @@ final class ImportCache { /// Calls [importer.canonicalize] and prints a deprecation warning if it /// returns a relative URL. - Uri? _canonicalize(Importer importer, Uri url, bool forImport) { - var result = (forImport - ? inImportRule(() => importer.canonicalize(url)) - : importer.canonicalize(url)); - if (result?.scheme == '') { - _logger.warnForDeprecation(Deprecation.relativeCanonical, """ -Importer $importer canonicalized $url to $result. -Relative canonical URLs are deprecated and will eventually be disallowed. -"""); + /// + /// If [resolveUrl] is `true`, this resolves [url] relative to [baseUrl] + /// before passing it to [importer]. + CanonicalizeResult? _canonicalize( + Importer importer, Uri url, Uri? baseUrl, bool forImport, + {bool resolveUrl = false}) { + var resolved = + resolveUrl && baseUrl != null ? baseUrl.resolveUri(url) : url; + var canonicalize = forImport + ? () => inImportRule(() => importer.canonicalize(resolved)) + : () => importer.canonicalize(resolved); + + var passContainingUrl = baseUrl != null && + (url.scheme == '' || importer.isNonCanonicalScheme(url.scheme)); + var result = + withContainingUrl(passContainingUrl ? baseUrl : null, canonicalize); + if (result == null) return null; + + if (result.scheme == '') { + _logger.warnForDeprecation( + Deprecation.relativeCanonical, + "Importer $importer canonicalized $resolved to $result.\n" + "Relative canonical URLs are deprecated and will eventually be " + "disallowed."); + } else if (importer.isNonCanonicalScheme(result.scheme)) { + throw "Importer $importer canonicalized $resolved to $result, which " + "uses a scheme declared as non-canonical."; } - return result; + + return (importer, result, originalUrl: resolved); } /// Tries to import [url] using one of this cache's importers. diff --git a/lib/src/importer.dart b/lib/src/importer.dart index 12d626a21..04a08c0f8 100644 --- a/lib/src/importer.dart +++ b/lib/src/importer.dart @@ -40,4 +40,6 @@ abstract class Importer extends AsyncImporter { DateTime modificationTime(Uri url) => DateTime.now(); bool couldCanonicalize(Uri url, Uri canonicalUrl) => true; + + bool isNonCanonicalScheme(String scheme) => false; } diff --git a/lib/src/importer/async.dart b/lib/src/importer/async.dart index 912d05084..d7d6951fe 100644 --- a/lib/src/importer/async.dart +++ b/lib/src/importer/async.dart @@ -41,6 +41,21 @@ abstract class AsyncImporter { @nonVirtual bool get fromImport => utils.fromImport; + /// The canonical URL of the stylesheet that caused the current [canonicalize] + /// invocation. + /// + /// This is only set when the containing stylesheet has a canonical URL, and + /// when the URL being canonicalized is either relative or has a scheme for + /// which [isNonCanonicalScheme] returns `true`. This restriction ensures that + /// canonical URLs are always interpreted the same way regardless of their + /// context. + /// + /// Subclasses should only access this from within calls to [canonicalize]. + /// Outside of that context, its value is undefined and subject to change. + @protected + @nonVirtual + Uri? get containingUrl => utils.containingUrl; + /// If [url] is recognized by this importer, returns its canonical format. /// /// Note that canonical URLs *must* be absolute, including a scheme. Returning @@ -137,4 +152,16 @@ abstract class AsyncImporter { /// [url] would actually resolve to [canonicalUrl]. Subclasses are not allowed /// to return false negatives. FutureOr couldCanonicalize(Uri url, Uri canonicalUrl) => true; + + /// Returns whether the given URL scheme (without `:`) should be considered + /// "non-canonical" for this importer. + /// + /// An importer may not return a URL with a non-canonical scheme from + /// [canonicalize]. In exchange, [containingUrl] is available within + /// [canonicalize] for absolute URLs with non-canonical schemes so that the + /// importer can resolve those URLs differently based on where they're loaded. + /// + /// This must always return the same value for the same [scheme]. It is + /// expected to be very efficient. + FutureOr isNonCanonicalScheme(String scheme) => false; } diff --git a/lib/src/importer/js_to_dart/async.dart b/lib/src/importer/js_to_dart/async.dart index 529789d11..11ffbd735 100644 --- a/lib/src/importer/js_to_dart/async.dart +++ b/lib/src/importer/js_to_dart/async.dart @@ -14,21 +14,35 @@ import '../../js/utils.dart'; import '../../util/nullable.dart'; import '../async.dart'; import '../result.dart'; +import 'utils.dart'; /// A wrapper for a potentially-asynchronous JS API importer that exposes it as /// a Dart [AsyncImporter]. final class JSToDartAsyncImporter extends AsyncImporter { /// The wrapped canonicalize function. - final Object? Function(String, CanonicalizeOptions) _canonicalize; + final Object? Function(String, CanonicalizeContext) _canonicalize; /// The wrapped load function. final Object? Function(JSUrl) _load; - JSToDartAsyncImporter(this._canonicalize, this._load); + /// The set of URL schemes that this importer promises never to return from + /// [canonicalize]. + final Set _nonCanonicalSchemes; + + JSToDartAsyncImporter( + this._canonicalize, this._load, Iterable? nonCanonicalSchemes) + : _nonCanonicalSchemes = nonCanonicalSchemes == null + ? const {} + : Set.unmodifiable(nonCanonicalSchemes) { + _nonCanonicalSchemes.forEach(validateUrlScheme); + } FutureOr canonicalize(Uri url) async { var result = wrapJSExceptions(() => _canonicalize( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; @@ -42,7 +56,7 @@ final class JSToDartAsyncImporter extends AsyncImporter { if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; - result as NodeImporterResult; + result as JSImporterResult; var contents = result.contents; if (!isJsString(contents)) { jsThrow(ArgumentError.value(contents, 'contents', @@ -59,4 +73,7 @@ final class JSToDartAsyncImporter extends AsyncImporter { syntax: parseSyntax(syntax), sourceMapUrl: result.sourceMapUrl.andThen(jsToDartUrl)); } + + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); } diff --git a/lib/src/importer/js_to_dart/async_file.dart b/lib/src/importer/js_to_dart/async_file.dart index 374783f08..e984531dc 100644 --- a/lib/src/importer/js_to_dart/async_file.dart +++ b/lib/src/importer/js_to_dart/async_file.dart @@ -11,6 +11,7 @@ import 'package:node_interop/util.dart'; import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; +import '../../util/nullable.dart'; import '../async.dart'; import '../filesystem.dart'; import '../result.dart'; @@ -26,7 +27,7 @@ final _filesystemImporter = FilesystemImporter('.'); /// it as a Dart [AsyncImporter]. final class JSToDartAsyncFileImporter extends AsyncImporter { /// The wrapped `findFileUrl` function. - final Object? Function(String, CanonicalizeOptions) _findFileUrl; + final Object? Function(String, CanonicalizeContext) _findFileUrl; JSToDartAsyncFileImporter(this._findFileUrl); @@ -34,7 +35,10 @@ final class JSToDartAsyncFileImporter extends AsyncImporter { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (isPromise(result)) result = await promiseToFuture(result as Promise); if (result == null) return null; if (!isJSUrl(result)) { @@ -58,4 +62,6 @@ final class JSToDartAsyncFileImporter extends AsyncImporter { bool couldCanonicalize(Uri url, Uri canonicalUrl) => _filesystemImporter.couldCanonicalize(url, canonicalUrl); + + bool isNonCanonicalScheme(String scheme) => scheme != 'file'; } diff --git a/lib/src/importer/js_to_dart/file.dart b/lib/src/importer/js_to_dart/file.dart index 3772e87f5..9ad474d00 100644 --- a/lib/src/importer/js_to_dart/file.dart +++ b/lib/src/importer/js_to_dart/file.dart @@ -9,6 +9,7 @@ import '../../importer.dart'; import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; +import '../../util/nullable.dart'; import '../utils.dart'; /// A filesystem importer to use for most implementation details of @@ -21,7 +22,7 @@ final _filesystemImporter = FilesystemImporter('.'); /// it as a Dart [AsyncImporter]. final class JSToDartFileImporter extends Importer { /// The wrapped `findFileUrl` function. - final Object? Function(String, CanonicalizeOptions) _findFileUrl; + final Object? Function(String, CanonicalizeContext) _findFileUrl; JSToDartFileImporter(this._findFileUrl); @@ -29,7 +30,10 @@ final class JSToDartFileImporter extends Importer { if (url.scheme == 'file') return _filesystemImporter.canonicalize(url); var result = wrapJSExceptions(() => _findFileUrl( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (result == null) return null; if (isPromise(result)) { @@ -57,4 +61,6 @@ final class JSToDartFileImporter extends Importer { bool couldCanonicalize(Uri url, Uri canonicalUrl) => _filesystemImporter.couldCanonicalize(url, canonicalUrl); + + bool isNonCanonicalScheme(String scheme) => scheme != 'file'; } diff --git a/lib/src/importer/js_to_dart/sync.dart b/lib/src/importer/js_to_dart/sync.dart index da05420ed..f69dafb35 100644 --- a/lib/src/importer/js_to_dart/sync.dart +++ b/lib/src/importer/js_to_dart/sync.dart @@ -10,21 +10,35 @@ import '../../js/importer.dart'; import '../../js/url.dart'; import '../../js/utils.dart'; import '../../util/nullable.dart'; +import 'utils.dart'; /// A wrapper for a synchronous JS API importer that exposes it as a Dart /// [Importer]. final class JSToDartImporter extends Importer { /// The wrapped canonicalize function. - final Object? Function(String, CanonicalizeOptions) _canonicalize; + final Object? Function(String, CanonicalizeContext) _canonicalize; /// The wrapped load function. final Object? Function(JSUrl) _load; - JSToDartImporter(this._canonicalize, this._load); + /// The set of URL schemes that this importer promises never to return from + /// [canonicalize]. + final Set _nonCanonicalSchemes; + + JSToDartImporter( + this._canonicalize, this._load, Iterable? nonCanonicalSchemes) + : _nonCanonicalSchemes = nonCanonicalSchemes == null + ? const {} + : Set.unmodifiable(nonCanonicalSchemes) { + _nonCanonicalSchemes.forEach(validateUrlScheme); + } Uri? canonicalize(Uri url) { var result = wrapJSExceptions(() => _canonicalize( - url.toString(), CanonicalizeOptions(fromImport: fromImport))); + url.toString(), + CanonicalizeContext( + fromImport: fromImport, + containingUrl: containingUrl.andThen(dartToJSUrl)))); if (result == null) return null; if (isJSUrl(result)) return jsToDartUrl(result as JSUrl); @@ -47,7 +61,7 @@ final class JSToDartImporter extends Importer { "functions.")); } - result as NodeImporterResult; + result as JSImporterResult; var contents = result.contents; if (!isJsString(contents)) { jsThrow(ArgumentError.value(contents, 'contents', @@ -64,4 +78,7 @@ final class JSToDartImporter extends Importer { syntax: parseSyntax(syntax), sourceMapUrl: result.sourceMapUrl.andThen(jsToDartUrl)); } + + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); } diff --git a/lib/src/importer/js_to_dart/utils.dart b/lib/src/importer/js_to_dart/utils.dart new file mode 100644 index 000000000..6cd2106f0 --- /dev/null +++ b/lib/src/importer/js_to_dart/utils.dart @@ -0,0 +1,16 @@ +// Copyright 2023 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'package:node_interop/js.dart'; + +import '../../js/utils.dart'; +import '../utils.dart'; + +/// Throws a JsError if [scheme] isn't a valid URL scheme. +void validateUrlScheme(String scheme) { + if (!isValidUrlScheme(scheme)) { + jsThrow( + JsError('"$scheme" isn\'t a valid URL scheme (for example "file").')); + } +} diff --git a/lib/src/importer/utils.dart b/lib/src/importer/utils.dart index 464a050f8..a68ae6f5e 100644 --- a/lib/src/importer/utils.dart +++ b/lib/src/importer/utils.dart @@ -17,11 +17,29 @@ import '../io.dart'; /// removed, at which point we can delete this and have one consistent behavior. bool get fromImport => Zone.current[#_inImportRule] as bool? ?? false; +/// The URL of the stylesheet that contains the current load. +Uri? get containingUrl => switch (Zone.current[#_containingUrl]) { + null => throw StateError( + "containingUrl may only be accessed within a call to canonicalize()."), + #_none => null, + Uri url => url, + var value => throw StateError( + "Unexpected Zone.current[#_containingUrl] value $value.") + }; + /// Runs [callback] in a context where [fromImport] returns `true` and /// [resolveImportPath] uses `@import` semantics rather than `@use` semantics. T inImportRule(T callback()) => runZoned(callback, zoneValues: {#_inImportRule: true}); +/// Runs [callback] in a context where [containingUrl] returns [url]. +/// +/// If [when] is `false`, runs [callback] without setting [containingUrl]. +T withContainingUrl(Uri? url, T callback()) => + // Use #_none as a sentinel value so we can distinguish a containing URL + // that's set to null from one that's unset at all. + runZoned(callback, zoneValues: {#_containingUrl: url ?? #_none}); + /// Resolves an imported path using the same logic as the filesystem importer. /// /// This tries to fill in extensions and partial prefixes and check for a @@ -82,3 +100,9 @@ String? _exactlyOne(List paths) => switch (paths) { /// /// Otherwise, returns `null`. T? _ifInImport(T callback()) => fromImport ? callback() : null; + +/// A regular expression matching valid URL schemes. +final _urlSchemeRegExp = RegExp(r"^[a-z0-9+.-]+$"); + +/// Returns whether [scheme] is a valid URL scheme. +bool isValidUrlScheme(String scheme) => _urlSchemeRegExp.hasMatch(scheme); diff --git a/lib/src/js/compile.dart b/lib/src/js/compile.dart index af5702308..0e734dc77 100644 --- a/lib/src/js/compile.dart +++ b/lib/src/js/compile.dart @@ -184,7 +184,7 @@ OutputStyle _parseOutputStyle(String? style) => switch (style) { AsyncImporter _parseAsyncImporter(Object? importer) { if (importer == null) jsThrow(JsError("Importers may not be null.")); - importer as NodeImporter; + importer as JSImporter; var canonicalize = importer.canonicalize; var load = importer.load; if (importer.findFileUrl case var findFileUrl?) { @@ -200,7 +200,8 @@ AsyncImporter _parseAsyncImporter(Object? importer) { "An importer must have either canonicalize and load methods, or a " "findFileUrl method.")); } else { - return JSToDartAsyncImporter(canonicalize, load); + return JSToDartAsyncImporter(canonicalize, load, + _normalizeNonCanonicalSchemes(importer.nonCanonicalScheme)); } } @@ -208,7 +209,7 @@ AsyncImporter _parseAsyncImporter(Object? importer) { Importer _parseImporter(Object? importer) { if (importer == null) jsThrow(JsError("Importers may not be null.")); - importer as NodeImporter; + importer as JSImporter; var canonicalize = importer.canonicalize; var load = importer.load; if (importer.findFileUrl case var findFileUrl?) { @@ -224,10 +225,23 @@ Importer _parseImporter(Object? importer) { "An importer must have either canonicalize and load methods, or a " "findFileUrl method.")); } else { - return JSToDartImporter(canonicalize, load); + return JSToDartImporter(canonicalize, load, + _normalizeNonCanonicalSchemes(importer.nonCanonicalScheme)); } } +/// Converts a JS-style `nonCanonicalScheme` field into the form expected by +/// Dart classes. +List? _normalizeNonCanonicalSchemes(Object? schemes) => + switch (schemes) { + String scheme => [scheme], + List schemes => schemes.cast(), + null => null, + _ => jsThrow( + JsError('nonCanonicalScheme must be a string or list of strings, was ' + '"$schemes"')) + }; + /// Implements the simplification algorithm for custom function return `Value`s. /// {@link https://github.com/sass/sass/blob/main/spec/types/calculation.md#simplifying-a-calculationvalue} Value _simplifyValue(Value value) => switch (value) { diff --git a/lib/src/js/compile_options.dart b/lib/src/js/compile_options.dart index 603adba4e..1789539d5 100644 --- a/lib/src/js/compile_options.dart +++ b/lib/src/js/compile_options.dart @@ -30,5 +30,5 @@ class CompileOptions { class CompileStringOptions extends CompileOptions { external String? get syntax; external JSUrl? get url; - external NodeImporter? get importer; + external JSImporter? get importer; } diff --git a/lib/src/js/importer.dart b/lib/src/js/importer.dart index 3be952951..0469737ee 100644 --- a/lib/src/js/importer.dart +++ b/lib/src/js/importer.dart @@ -8,23 +8,25 @@ import 'url.dart'; @JS() @anonymous -class NodeImporter { - external Object? Function(String, CanonicalizeOptions)? get canonicalize; +class JSImporter { + external Object? Function(String, CanonicalizeContext)? get canonicalize; external Object? Function(JSUrl)? get load; - external Object? Function(String, CanonicalizeOptions)? get findFileUrl; + external Object? Function(String, CanonicalizeContext)? get findFileUrl; + external Object? get nonCanonicalScheme; } @JS() @anonymous -class CanonicalizeOptions { +class CanonicalizeContext { external bool get fromImport; + external JSUrl? get containingUrl; - external factory CanonicalizeOptions({bool fromImport}); + external factory CanonicalizeContext({bool fromImport, JSUrl? containingUrl}); } @JS() @anonymous -class NodeImporterResult { +class JSImporterResult { external String? get contents; external String? get syntax; external JSUrl? get sourceMapUrl; diff --git a/lib/src/util/span.dart b/lib/src/util/span.dart index 46866f60d..bcb9b6165 100644 --- a/lib/src/util/span.dart +++ b/lib/src/util/span.dart @@ -9,8 +9,10 @@ import 'package:string_scanner/string_scanner.dart'; import '../utils.dart'; import 'character.dart'; -/// A span that points nowhere, only used for fake AST nodes that will never be -/// presented to the user. +/// A span that points nowhere. +/// +/// This is used for fake AST nodes that will never be presented to the user, as +/// well as for embedded compilation failures that have no associated spans. final bogusSpan = SourceFile.decoded([]).span(0); extension SpanExtensions on FileSpan { diff --git a/test/dart_api/importer_test.dart b/test/dart_api/importer_test.dart index 113e9a24b..28ec53bee 100644 --- a/test/dart_api/importer_test.dart +++ b/test/dart_api/importer_test.dart @@ -112,6 +112,99 @@ void main() { }); }); + group("the containing URL", () { + test("is null for a potentially canonical scheme", () { + late TestImporter importer; + compileString('@import "u:orange";', + importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, isNull); + return url; + }), (_) => ImporterResult('', indented: false)) + ], + url: 'x:original.scss'); + }); + + test("throws an error outside canonicalize", () { + late TestImporter importer; + compileString('@import "orange";', importers: [ + importer = + TestImporter((url) => Uri.parse("u:$url"), expectAsync1((url) { + expect(() => importer.publicContainingUrl, throwsStateError); + return ImporterResult('', indented: false); + })) + ]); + }); + + group("for a non-canonical scheme", () { + test("is set to the original URL", () { + late TestImporter importer; + compileString('@import "u:orange";', + importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, + equals(Uri.parse('x:original.scss'))); + return url.replace(scheme: 'x'); + }), (_) => ImporterResult('', indented: false), + nonCanonicalSchemes: {'u'}) + ], + url: 'x:original.scss'); + }); + + test("is null if the original URL is null", () { + late TestImporter importer; + compileString('@import "u:orange";', importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, isNull); + return url.replace(scheme: 'x'); + }), (_) => ImporterResult('', indented: false), + nonCanonicalSchemes: {'u'}) + ]); + }); + }); + + group("for a schemeless load", () { + test("is set to the original URL", () { + late TestImporter importer; + compileString('@import "orange";', + importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, + equals(Uri.parse('x:original.scss'))); + return Uri.parse("u:$url"); + }), (_) => ImporterResult('', indented: false)) + ], + url: 'x:original.scss'); + }); + + test("is null if the original URL is null", () { + late TestImporter importer; + compileString('@import "orange";', importers: [ + importer = TestImporter(expectAsync1((url) { + expect(importer.publicContainingUrl, isNull); + return Uri.parse("u:$url"); + }), (_) => ImporterResult('', indented: false)) + ]); + }); + }); + }); + + test( + "throws an error if the importer returns a canonical URL with a " + "non-canonical scheme", () { + expect( + () => compileString('@import "orange";', importers: [ + TestImporter(expectAsync1((url) => Uri.parse("u:$url")), + (_) => ImporterResult('', indented: false), + nonCanonicalSchemes: {'u'}) + ]), throwsA(predicate((error) { + expect(error, const TypeMatcher()); + expect(error.toString(), + contains("uses a scheme declared as non-canonical")); + return true; + }))); + }); + test("uses an importer's source map URL", () { var result = compileStringToResult('@import "orange";', importers: [ diff --git a/test/dart_api/test_importer.dart b/test/dart_api/test_importer.dart index 87af315df..fda884c93 100644 --- a/test/dart_api/test_importer.dart +++ b/test/dart_api/test_importer.dart @@ -9,10 +9,22 @@ import 'package:sass/sass.dart'; class TestImporter extends Importer { final Uri? Function(Uri url) _canonicalize; final ImporterResult? Function(Uri url) _load; + final Set _nonCanonicalSchemes; - TestImporter(this._canonicalize, this._load); + /// Public access to the containing URL so that [canonicalize] and [load] + /// implementations can access them. + Uri? get publicContainingUrl => containingUrl; + + TestImporter(this._canonicalize, this._load, + {Iterable? nonCanonicalSchemes}) + : _nonCanonicalSchemes = nonCanonicalSchemes == null + ? const {} + : Set.unmodifiable(nonCanonicalSchemes); Uri? canonicalize(Uri url) => _canonicalize(url); ImporterResult? load(Uri url) => _load(url); + + bool isNonCanonicalScheme(String scheme) => + _nonCanonicalSchemes.contains(scheme); } diff --git a/test/embedded/importer_test.dart b/test/embedded/importer_test.dart index 71574cb4e..fdfc904d2 100644 --- a/test/embedded/importer_test.dart +++ b/test/embedded/importer_test.dart @@ -64,6 +64,46 @@ void main() { process, errorId, "Missing mandatory field Importer.importer"); await process.shouldExit(76); }); + + group("for an importer with nonCanonicalScheme set:", () { + test("path", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + path: "somewhere", nonCanonicalScheme: ["u"]) + ])); + await expectParamsError( + process, + errorId, + "Importer.non_canonical_scheme may only be set along with " + "Importer.importer.importer_id"); + await process.shouldExit(76); + }); + + test("file importer", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + fileImporterId: 1, nonCanonicalScheme: ["u"]) + ])); + await expectParamsError( + process, + errorId, + "Importer.non_canonical_scheme may only be set along with " + "Importer.importer.importer_id"); + await process.shouldExit(76); + }); + + test("unset", () async { + process.send(compileString("a {b: c}", + importer: InboundMessage_CompileRequest_Importer( + nonCanonicalScheme: ["u"]))); + await expectParamsError( + process, + errorId, + "Importer.non_canonical_scheme may only be set along with " + "Importer.importer.importer_id"); + await process.shouldExit(76); + }); + }); }); group("canonicalization", () { @@ -156,6 +196,86 @@ void main() { await process.close(); }); + group("the containing URL", () { + test("is unset for a potentially canonical scheme", () async { + process.send(compileString('@import "u:orange"', importers: [ + InboundMessage_CompileRequest_Importer(importerId: 1) + ])); + + var request = await getCanonicalizeRequest(process); + expect(request.hasContainingUrl(), isFalse); + await process.close(); + }); + + group("for a non-canonical scheme", () { + test("is set to the original URL", () async { + process.send(compileString('@import "u:orange"', + importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u"]) + ], + url: "x:original.scss")); + + var request = await getCanonicalizeRequest(process); + expect(request.containingUrl, equals("x:original.scss")); + await process.close(); + }); + + test("is unset to the original URL is unset", () async { + process.send(compileString('@import "u:orange"', importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u"]) + ])); + + var request = await getCanonicalizeRequest(process); + expect(request.hasContainingUrl(), isFalse); + await process.close(); + }); + }); + + group("for a schemeless load", () { + test("is set to the original URL", () async { + process.send(compileString('@import "orange"', + importers: [ + InboundMessage_CompileRequest_Importer(importerId: 1) + ], + url: "x:original.scss")); + + var request = await getCanonicalizeRequest(process); + expect(request.containingUrl, equals("x:original.scss")); + await process.close(); + }); + + test("is unset to the original URL is unset", () async { + process.send(compileString('@import "u:orange"', importers: [ + InboundMessage_CompileRequest_Importer(importerId: 1) + ])); + + var request = await getCanonicalizeRequest(process); + expect(request.hasContainingUrl(), isFalse); + await process.close(); + }); + }); + }); + + test( + "fails if the importer returns a canonical URL with a non-canonical " + "scheme", () async { + process.send(compileString("@import 'other'", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u"]) + ])); + + var request = await getCanonicalizeRequest(process); + process.send(InboundMessage( + canonicalizeResponse: InboundMessage_CanonicalizeResponse( + id: request.id, url: "u:other"))); + + await _expectImportError( + process, contains('a scheme declared as non-canonical')); + await process.close(); + }); + test("attempts importers in order", () async { process.send(compileString("@import 'other'", importers: [ for (var i = 0; i < 10; i++) @@ -474,6 +594,44 @@ void main() { await process.close(); }); }); + + group("fails compilation for an invalid scheme:", () { + test("empty", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: [""]) + ])); + + var failure = await getCompileFailure(process); + expect(failure.message, + equals('"" isn\'t a valid URL scheme (for example "file").')); + await process.close(); + }); + + test("uppercase", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["U"]) + ])); + + var failure = await getCompileFailure(process); + expect(failure.message, + equals('"U" isn\'t a valid URL scheme (for example "file").')); + await process.close(); + }); + + test("colon", () async { + process.send(compileString("a {b: c}", importers: [ + InboundMessage_CompileRequest_Importer( + importerId: 1, nonCanonicalScheme: ["u:"]) + ])); + + var failure = await getCompileFailure(process); + expect(failure.message, + equals('"u:" isn\'t a valid URL scheme (for example "file").')); + await process.close(); + }); + }); } /// Handles a `CanonicalizeRequest` and returns a response with a generic diff --git a/tool/grind/utils.dart b/tool/grind/utils.dart index 12fd8ed1e..21d9f66c8 100644 --- a/tool/grind/utils.dart +++ b/tool/grind/utils.dart @@ -58,6 +58,10 @@ String cloneOrCheckout(String url, String ref, {String? name}) { } var path = p.join("build", name); + if (Link(path).existsSync()) { + log("$url is symlinked, leaving as-is"); + return path; + } if (!Directory(p.join(path, '.git')).existsSync()) { delete(Directory(path));