fix(compiler-cli): handle pseudo cycles in inline source-maps (#40435)

When a source-map has an inline source, any source-map linked from
that source should only be loaded if itself is also inline; it should not
attempt to load a source-map from the file-system. Otherwise we can
find ourselves with inadvertent infinite cyclic dependencies.

For example, if a transpiler takes a file (e.g. index.js) and generates
a new file overwriting the original file - capturing the original
source inline in the new source-map (index.js.map) - the source
file loader might read the inline original file (also index.js) and
then try to load the `index.js.map` file from disk - ad infinitum.

Note that the first call to `loadSourceFile()` is special, since you can
pass in the source-file and source-map contents directly as in-memory
strrngs. This is common if the transpiler has just generated these and has
not yet written them to disk.
When the contents are passed into `loadSourceFile()` directly, they are
not treated as "inline" for the purposes described above since there is
no chance of these "in-memory" source and source-map contents being caught
up in a cyclic dependency.

Fixes #40408

PR Close #40435

packages/compiler-cli/ngcc/src/rendering/source_maps.ts

@@ -11,7 +11,7 @@ import * as ts from 'typescript';
 
 import {absoluteFrom, absoluteFromSourceFile, ReadonlyFileSystem} from '../../../src/ngtsc/file_system';
 import {Logger} from '../../../src/ngtsc/logging';
-import {RawSourceMap, SourceFileLoader} from '../../../src/ngtsc/sourcemaps';
+import {ContentOrigin, RawSourceMap, SourceFileLoader} from '../../../src/ngtsc/sourcemaps';
 
 import {FileToWrite} from './utils';
 
@@ -28,22 +28,22 @@ export interface SourceMapInfo {
 export function renderSourceAndMap(
     logger: Logger, fs: ReadonlyFileSystem, sourceFile: ts.SourceFile,
     generatedMagicString: MagicString): FileToWrite[] {
-  const generatedPath = absoluteFromSourceFile(sourceFile);
-  const generatedMapPath = absoluteFrom(`${generatedPath}.map`);
+  const sourceFilePath = absoluteFromSourceFile(sourceFile);
+  const sourceMapPath = absoluteFrom(`${sourceFilePath}.map`);
   const generatedContent = generatedMagicString.toString();
   const generatedMap: RawSourceMap = generatedMagicString.generateMap(
-      {file: generatedPath, source: generatedPath, includeContent: true});
+      {file: sourceFilePath, source: sourceFilePath, includeContent: true});
 
   try {
     const loader = new SourceFileLoader(fs, logger, {});
     const generatedFile = loader.loadSourceFile(
-        generatedPath, generatedContent, {map: generatedMap, mapPath: generatedMapPath});
+        sourceFilePath, generatedContent, {map: generatedMap, mapPath: sourceMapPath});
 
     const rawMergedMap: RawSourceMap = generatedFile.renderFlattenedSourceMap();
     const mergedMap = fromObject(rawMergedMap);
-    const firstSource = generatedFile.sources[0];
-    if (firstSource && (firstSource.rawMap !== null || !sourceFile.isDeclarationFile) &&
-        firstSource.inline) {
+    const originalFile = loader.loadSourceFile(sourceFilePath, generatedMagicString.original);
+    if (originalFile.rawMap === null && !sourceFile.isDeclarationFile ||
+        originalFile.rawMap?.origin === ContentOrigin.Inline) {
       // We render an inline source map if one of:
       // * there was no input source map and this is not a typings file;
       // * the input source map exists and was inline.
@@ -52,21 +52,21 @@ export function renderSourceAndMap(
       // the input file because these inline source maps can be very large and it impacts on the
       // performance of IDEs that need to read them to provide intellisense etc.
       return [
-        {path: generatedPath, contents: `${generatedFile.contents}\n${mergedMap.toComment()}`}
-      ];
-    } else {
-      const sourceMapComment = generateMapFileComment(`${fs.basename(generatedPath)}.map`);
-      return [
-        {path: generatedPath, contents: `${generatedFile.contents}\n${sourceMapComment}`},
-        {path: generatedMapPath, contents: mergedMap.toJSON()}
+        {path: sourceFilePath, contents: `${generatedFile.contents}\n${mergedMap.toComment()}`}
       ];
     }
+
+    const sourceMapComment = generateMapFileComment(`${fs.basename(sourceFilePath)}.map`);
+    return [
+      {path: sourceFilePath, contents: `${generatedFile.contents}\n${sourceMapComment}`},
+      {path: sourceMapPath, contents: mergedMap.toJSON()}
+    ];
   } catch (e) {
-    logger.error(`Error when flattening the source-map "${generatedMapPath}" for "${
-        generatedPath}": ${e.toString()}`);
+    logger.error(`Error when flattening the source-map "${sourceMapPath}" for "${
+        sourceFilePath}": ${e.toString()}`);
     return [
-      {path: generatedPath, contents: generatedContent},
-      {path: generatedMapPath, contents: fromObject(generatedMap).toJSON()},
+      {path: sourceFilePath, contents: generatedContent},
+      {path: sourceMapPath, contents: fromObject(generatedMap).toJSON()},
     ];
   }
 }

packages/compiler-cli/src/ngtsc/sourcemaps/index.ts

@@ -5,6 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-export {RawSourceMap} from './src/raw_source_map';
+export {ContentOrigin} from './src/content_origin';
+export {MapAndPath, RawSourceMap} from './src/raw_source_map';
 export {Mapping, SourceFile} from './src/source_file';
-export {MapAndPath, SourceFileLoader} from './src/source_file_loader';
+export {SourceFileLoader} from './src/source_file_loader';

packages/compiler-cli/src/ngtsc/sourcemaps/src/content_origin.ts

@@ -0,0 +1,34 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+/**
+ * From where the content for a source file or source-map came.
+ *
+ * - Source files can be linked to source-maps by:
+ *   - providing the content inline via a base64 encoded data comment,
+ *   - providing a URL to the file path in a comment,
+ *   - the loader inferring the source-map path from the source file path.
+ * - Source-maps can link to source files by:
+ *   - providing the content inline in the `sourcesContent` property
+ *   - providing the path to the file in the `sources` property
+ */
+export enum ContentOrigin {
+  /**
+   * The contents were provided programmatically when calling `loadSourceFile()`.
+   */
+  Provided,
+  /**
+   * The contents were extracted directly form the contents of the referring file.
+   */
+  Inline,
+  /**
+   * The contents were loaded from the file-system, after being explicitly referenced or inferred
+   * from the referring file.
+   */
+  FileSystem,
+}

packages/compiler-cli/src/ngtsc/sourcemaps/src/raw_source_map.ts

@@ -5,6 +5,8 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
+import {AbsoluteFsPath} from '../../file_system';
+import {ContentOrigin} from './content_origin';
 
 /**
  * This interface is the basic structure of the JSON in a raw source map that one might load from
@@ -19,3 +21,22 @@ export interface RawSourceMap {
   sourcesContent?: (string|null)[];
   mappings: string;
 }
+
+
+/**
+ * The path and content of a source-map.
+ */
+export interface MapAndPath {
+  /** The path to the source map if it was external or `null` if it was inline. */
+  mapPath: AbsoluteFsPath|null;
+  /** The raw source map itself. */
+  map: RawSourceMap;
+}
+
+/**
+ * Information about a loaded source-map.
+ */
+export interface SourceMapInfo extends MapAndPath {
+  /** From where the content for this source-map came. */
+  origin: ContentOrigin;
+}

packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file.ts

@@ -10,7 +10,7 @@ import {decode, encode, SourceMapMappings, SourceMapSegment} from 'sourcemap-cod
 
 import {AbsoluteFsPath, PathManipulation} from '../../file_system';
 
-import {RawSourceMap} from './raw_source_map';
+import {RawSourceMap, SourceMapInfo} from './raw_source_map';
 import {compareSegments, offsetSegment, SegmentMarker} from './segment_marker';
 
 export function removeSourceMapComments(contents: string): string {
@@ -33,10 +33,8 @@ export class SourceFile {
       readonly sourcePath: AbsoluteFsPath,
       /** The contents of this source file. */
       readonly contents: string,
-      /** The raw source map (if any) associated with this source file. */
-      readonly rawMap: RawSourceMap|null,
-      /** Whether this source file's source map was inline or external. */
-      readonly inline: boolean,
+      /** The raw source map (if any) referenced by this source file. */
+      readonly rawMap: SourceMapInfo|null,
       /** Any source files referenced by the raw source map associated with this source file. */
       readonly sources: (SourceFile|null)[],
       private fs: PathManipulation,
@@ -141,7 +139,8 @@ export class SourceFile {
    * source files with no transitive source maps.
    */
   private flattenMappings(): Mapping[] {
-    const mappings = parseMappings(this.rawMap, this.sources, this.startOfLinePositions);
+    const mappings =
+        parseMappings(this.rawMap && this.rawMap.map, this.sources, this.startOfLinePositions);
     ensureOriginalSegmentLinks(mappings);
     const flattenedMappings: Mapping[] = [];
     for (let mappingIndex = 0; mappingIndex < mappings.length; mappingIndex++) {

packages/compiler-cli/src/ngtsc/sourcemaps/src/source_file_loader.ts

@@ -10,7 +10,8 @@ import {commentRegex, fromComment, mapFileCommentRegex} from 'convert-source-map
 import {AbsoluteFsPath, ReadonlyFileSystem} from '../../file_system';
 import {Logger} from '../../logging';
 
-import {RawSourceMap} from './raw_source_map';
+import {ContentOrigin} from './content_origin';
+import {MapAndPath, RawSourceMap, SourceMapInfo} from './raw_source_map';
 import {SourceFile} from './source_file';
 
 const SCHEME_MATCHER = /^([a-z][a-z0-9.-]*):\/\//i;
@@ -33,32 +34,60 @@ export class SourceFileLoader {
       private schemeMap: Record<string, AbsoluteFsPath>) {}
 
   /**
-   * Load a source file, compute its source map, and recursively load any referenced source files.
+   * Load a source file from the provided content and source map, and recursively load any
+   * referenced source files.
    *
    * @param sourcePath The path to the source file to load.
    * @param contents The contents of the source file to load.
    * @param mapAndPath The raw source-map and the path to the source-map file.
    * @returns a SourceFile object created from the `contents` and provided source-map info.
    */
   loadSourceFile(sourcePath: AbsoluteFsPath, contents: string, mapAndPath: MapAndPath): SourceFile;
+  /**
+   * Load a source file from the provided content, compute its source map, and recursively load any
+   * referenced source files.
+   *
+   * @param sourcePath The path to the source file to load.
+   * @param contents The contents of the source file to load.
+   * @returns a SourceFile object created from the `contents` and computed source-map info.
+   */
+  loadSourceFile(sourcePath: AbsoluteFsPath, contents: string): SourceFile;
+  /**
+   * Load a source file from the file-system, compute its source map, and recursively load any
+   * referenced source files.
+   *
+   * @param sourcePath The path to the source file to load.
+   * @returns a SourceFile object if its contents could be loaded from disk, or null otherwise.
+   */
+  loadSourceFile(sourcePath: AbsoluteFsPath): SourceFile|null;
+  loadSourceFile(
+      sourcePath: AbsoluteFsPath, contents: string|null = null,
+      mapAndPath: MapAndPath|null = null): SourceFile|null {
+    const contentsOrigin = contents !== null ? ContentOrigin.Provided : ContentOrigin.FileSystem;
+    const sourceMapInfo: SourceMapInfo|null =
+        mapAndPath && {origin: ContentOrigin.Provided, ...mapAndPath};
+    return this.loadSourceFileInternal(sourcePath, contents, contentsOrigin, sourceMapInfo);
+  }
+
   /**
    * The overload used internally to load source files referenced in a source-map.
    *
    * In this case there is no guarantee that it will return a non-null SourceMap.
    *
    * @param sourcePath The path to the source file to load.
-   * @param contents The contents of the source file to load, if provided inline.
-   * If it is not known the contents will be read from the file at the `sourcePath`.
-   * @param mapAndPath The raw source-map and the path to the source-map file.
+   * @param contents The contents of the source file to load, if provided inline. If `null`,
+   *     the contents will be read from the file at the `sourcePath`.
+   * @param sourceOrigin Describes where the source content came from.
+   * @param sourceMapInfo The raw contents and path of the source-map file. If `null` the
+   *     source-map will be computed from the contents of the source file, either inline or loaded
+   *     from the file-system.
    *
-   * @returns a SourceFile if the content for one was provided or able to be loaded from disk,
+   * @returns a SourceFile if the content for one was provided or was able to be loaded from disk,
    * `null` otherwise.
    */
-  loadSourceFile(sourcePath: AbsoluteFsPath, contents?: string|null, mapAndPath?: null): SourceFile
-      |null;
-  loadSourceFile(
-      sourcePath: AbsoluteFsPath, contents: string|null = null,
-      mapAndPath: MapAndPath|null = null): SourceFile|null {
+  private loadSourceFileInternal(
+      sourcePath: AbsoluteFsPath, contents: string|null, sourceOrigin: ContentOrigin,
+      sourceMapInfo: SourceMapInfo|null): SourceFile|null {
     const previousPaths = this.currentPaths.slice();
     try {
       if (contents === null) {
@@ -69,21 +98,17 @@ export class SourceFileLoader {
       }
 
       // If not provided try to load the source map based on the source itself
-      if (mapAndPath === null) {
-        mapAndPath = this.loadSourceMap(sourcePath, contents);
+      if (sourceMapInfo === null) {
+        sourceMapInfo = this.loadSourceMap(sourcePath, contents, sourceOrigin);
       }
 
-      let map: RawSourceMap|null = null;
-      let inline = true;
       let sources: (SourceFile|null)[] = [];
-      if (mapAndPath !== null) {
-        const basePath = mapAndPath.mapPath || sourcePath;
-        sources = this.processSources(basePath, mapAndPath.map);
-        map = mapAndPath.map;
-        inline = mapAndPath.mapPath === null;
+      if (sourceMapInfo !== null) {
+        const basePath = sourceMapInfo.mapPath || sourcePath;
+        sources = this.processSources(basePath, sourceMapInfo);
       }
 
-      return new SourceFile(sourcePath, contents, map, inline, sources, this.fs);
+      return new SourceFile(sourcePath, contents, sourceMapInfo, sources, this.fs);
     } catch (e) {
       this.logger.warn(
           `Unable to fully load ${sourcePath} for source-map flattening: ${e.message}`);
@@ -100,23 +125,46 @@ export class SourceFileLoader {
    *
    * Source maps can be inline, as part of a base64 encoded comment, or external as a separate file
    * whose path is indicated in a comment or implied from the name of the source file itself.
+   *
+   * @param sourcePath the path to the source file.
+   * @param sourceContents the contents of the source file.
+   * @param sourceOrigin where the content of the source file came from.
+   * @returns the parsed contents and path of the source-map, if loading was successful, null
+   *     otherwise.
    */
-  private loadSourceMap(sourcePath: AbsoluteFsPath, contents: string): MapAndPath|null {
+  private loadSourceMap(
+      sourcePath: AbsoluteFsPath, sourceContents: string,
+      sourceOrigin: ContentOrigin): SourceMapInfo|null {
     // Only consider a source-map comment from the last non-empty line of the file, in case there
     // are embedded source-map comments elsewhere in the file (as can be the case with bundlers like
     // webpack).
-    const lastLine = this.getLastNonEmptyLine(contents);
+    const lastLine = this.getLastNonEmptyLine(sourceContents);
     const inline = commentRegex.exec(lastLine);
     if (inline !== null) {
-      return {map: fromComment(inline.pop()!).sourcemap, mapPath: null};
+      return {
+        map: fromComment(inline.pop()!).sourcemap,
+        mapPath: null,
+        origin: ContentOrigin.Inline,
+      };
+    }
+
+    if (sourceOrigin === ContentOrigin.Inline) {
+      // The source file was provided inline and its contents did not include an inline source-map.
+      // So we don't try to load an external source-map from the file-system, since this can lead to
+      // invalid circular dependencies.
+      return null;
     }
 
     const external = mapFileCommentRegex.exec(lastLine);
     if (external) {
       try {
         const fileName = external[1] || external[2];
         const externalMapPath = this.fs.resolve(this.fs.dirname(sourcePath), fileName);
-        return {map: this.readRawSourceMap(externalMapPath), mapPath: externalMapPath};
+        return {
+          map: this.readRawSourceMap(externalMapPath),
+          mapPath: externalMapPath,
+          origin: ContentOrigin.FileSystem,
+        };
       } catch (e) {
         this.logger.warn(
             `Unable to fully load ${sourcePath} for source-map flattening: ${e.message}`);
@@ -126,7 +174,11 @@ export class SourceFileLoader {
 
     const impliedMapPath = this.fs.resolve(sourcePath + '.map');
     if (this.fs.exists(impliedMapPath)) {
-      return {map: this.readRawSourceMap(impliedMapPath), mapPath: impliedMapPath};
+      return {
+        map: this.readRawSourceMap(impliedMapPath),
+        mapPath: impliedMapPath,
+        origin: ContentOrigin.FileSystem,
+      };
     }
 
     return null;
@@ -136,13 +188,23 @@ export class SourceFileLoader {
    * Iterate over each of the "sources" for this source file's source map, recursively loading each
    * source file and its associated source map.
    */
-  private processSources(basePath: AbsoluteFsPath, map: RawSourceMap): (SourceFile|null)[] {
+  private processSources(basePath: AbsoluteFsPath, {map, origin: sourceMapOrigin}: SourceMapInfo):
+      (SourceFile|null)[] {
     const sourceRoot = this.fs.resolve(
         this.fs.dirname(basePath), this.replaceSchemeWithPath(map.sourceRoot || ''));
     return map.sources.map((source, index) => {
       const path = this.fs.resolve(sourceRoot, this.replaceSchemeWithPath(source));
       const content = map.sourcesContent && map.sourcesContent[index] || null;
-      return this.loadSourceFile(path, content, null);
+      // The origin of this source file is "inline" if we extracted it from the source-map's
+      // `sourcesContent`, except when the source-map itself was "provided" in-memory.
+      // An inline source file is treated as if it were from the file-system if the source-map that
+      // contains it was provided in-memory. The first call to `loadSourceFile()` is special in that
+      // if you "provide" the contents of the source-map in-memory then we don't want to block
+      // loading sources from the file-system just because this source-map had an inline source.
+      const sourceOrigin = content !== null && sourceMapOrigin !== ContentOrigin.Provided ?
+          ContentOrigin.Inline :
+          ContentOrigin.FileSystem;
+      return this.loadSourceFileInternal(path, content, sourceOrigin, null);
     });
   }
 
@@ -206,11 +268,3 @@ export class SourceFileLoader {
         SCHEME_MATCHER, (_: string, scheme: string) => this.schemeMap[scheme.toLowerCase()] || '');
   }
 }
-
-/** A small helper structure that is returned from `loadSourceMap()`. */
-export interface MapAndPath {
-  /** The path to the source map if it was external or `null` if it was inline. */
-  mapPath: AbsoluteFsPath|null;
-  /** The raw source map itself. */
-  map: RawSourceMap;
-}