From 8e9b675133f84f65d749cb03b98c840d91c54cb3 Mon Sep 17 00:00:00 2001
From: Charles Lyding <19598772+clydin@users.noreply.github.com>
Date: Mon, 4 Dec 2023 11:23:15 -0500
Subject: [PATCH] fix(@angular-devkit/build-angular): check namespaced Sass
 variables when rebasing URLs

The `@use` Sass directive allows Sass variables to be accessed via a namespace. This was
previously not checked when performing URL path rebasing on imported Sass stylesheets.
These type of Sass variable references are now also ignored when attempting to rebase URL
paths during loading. The final rebased URL now also does not add a leading relative prefix
indicator (`./`) unless not already present.
---
 .../stylesheet-url-resolution_spec.ts         | 42 +++++++++++++++++++
 .../src/tools/sass/rebasing-importer.ts       | 39 +++++++++++++++--
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/packages/angular_devkit/build_angular/src/builders/application/tests/behavior/stylesheet-url-resolution_spec.ts b/packages/angular_devkit/build_angular/src/builders/application/tests/behavior/stylesheet-url-resolution_spec.ts
index 31481eab67b2..20edece4da69 100644
--- a/packages/angular_devkit/build_angular/src/builders/application/tests/behavior/stylesheet-url-resolution_spec.ts
+++ b/packages/angular_devkit/build_angular/src/builders/application/tests/behavior/stylesheet-url-resolution_spec.ts
@@ -163,5 +163,47 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
         }),
       );
     });
+
+    it('should not rebase a URL with a namespaced Sass variable reference', async () => {
+      await harness.writeFile(
+        'src/styles.scss',
+        `
+        @import "a";
+      `,
+      );
+      await harness.writeFile(
+        'src/a.scss',
+        `
+        @use './b' as named;
+        .a {
+          background-image: url(named.$my-var)
+        }
+      `,
+      );
+      await harness.writeFile(
+        'src/b.scss',
+        `
+        @forward './c.scss' show $my-var;
+      `,
+      );
+      await harness.writeFile(
+        'src/c.scss',
+        `
+        $my-var: "https://example.com/example.png";
+      `,
+      );
+
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        styles: ['src/styles.scss'],
+      });
+
+      const { result } = await harness.executeOnce();
+      expect(result?.success).toBe(true);
+
+      harness
+        .expectFile('dist/browser/styles.css')
+        .content.toContain('url(https://example.com/example.png)');
+    });
   });
 });
diff --git a/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts b/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts
index 7d657f3db12e..0c2f33d2d52b 100644
--- a/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts
+++ b/packages/angular_devkit/build_angular/src/tools/sass/rebasing-importer.ts
@@ -23,6 +23,28 @@ export interface DirectoryEntry {
   directories: Set<string>;
 }
 
+/**
+ * Ensures that a bare specifier URL path that is intended to be treated as
+ * a relative path has a leading `./` or `../` prefix.
+ *
+ * @param url A bare specifier URL path that should be considered relative.
+ * @returns
+ */
+function ensureRelative(url: string): string {
+  // Empty
+  if (!url) {
+    return url;
+  }
+
+  // Already relative
+  if (url[0] === '.' && (url[1] === '/' || (url[1] === '.' && url[2] === '/'))) {
+    return url;
+  }
+
+  // Needs prefix
+  return './' + url;
+}
+
 /**
  * A Sass Importer base class that provides the load logic to rebase all `url()` functions
  * within a stylesheet. The rebasing will ensure that the URLs in the output of the Sass compiler
@@ -55,8 +77,14 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
     // Rebase any URLs that are found
     let updatedContents;
     for (const { start, end, value } of findUrls(contents)) {
-      // Skip if value is empty, a Sass variable, or Webpack-specific prefix
-      if (value.length === 0 || value[0] === '$' || value[0] === '~' || value[0] === '^') {
+      // Skip if value is empty or Webpack-specific prefix
+      if (value.length === 0 || value[0] === '~' || value[0] === '^') {
+        continue;
+      }
+
+      // Skip if value is a Sass variable.
+      // Sass variable usage either starts with a `$` or contains a namespace and a `.$`
+      if (value[0] === '$' || /^\w+\.\$/.test(value)) {
         continue;
       }
 
@@ -69,10 +97,13 @@ abstract class UrlRebasingImporter implements Importer<'sync'> {
 
       // Normalize path separators and escape characters
       // https://developer.mozilla.org/en-US/docs/Web/CSS/url#syntax
-      const rebasedUrl = './' + rebasedPath.replace(/\\/g, '/').replace(/[()\s'"]/g, '\\$&');
+      const rebasedUrl = ensureRelative(
+        rebasedPath.replace(/\\/g, '/').replace(/[()\s'"]/g, '\\$&'),
+      );
 
       updatedContents ??= new MagicString(contents);
-      updatedContents.update(start, end, rebasedUrl);
+      // Always quote the URL to avoid potential downstream parsing problems
+      updatedContents.update(start, end, `"${rebasedUrl}"`);
     }
 
     if (updatedContents) {