honeymoose/Angular-Cn
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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
This commit is contained in:
Pete Bacon Darwin
2021-01-21 14:06:57 -08:00
committed by Jessica Janiuk
parent d482f5cdd3
commit dc06873c72
8 changed files with 509 additions and 266 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/compiler-cli/ngcc/src/rendering/source_maps.ts
+20 -20
View File
@@ -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()}`}
];
}
} catch (e) {
logger.error(`Error when flattening the source-map "${generatedMapPath}" for "${
generatedPath}": ${e.toString()}`);
const sourceMapComment = generateMapFileComment(`${fs.basename(sourceFilePath)}.map`);
return [
{path: generatedPath, contents: generatedContent},
{path: generatedMapPath, contents: fromObject(generatedMap).toJSON()},
{path: sourceFilePath, contents: `${generatedFile.contents}\n${sourceMapComment}`},
{path: sourceMapPath, contents: mergedMap.toJSON()}
];
} catch (e) {
logger.error(`Error when flattening the source-map "${sourceMapPath}" for "${
sourceFilePath}": ${e.toString()}`);
return [
{path: sourceFilePath, contents: generatedContent},
{path: sourceMapPath, contents: fromObject(generatedMap).toJSON()},
];
}
}
packages/compiler-cli/src/ngtsc/sourcemaps/index.ts
+3 -2
View File
@@ -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
+34
View File
@@ -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
+21
View File
@@ -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
+5 -6
View File
@@ -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
+90 -36
View File
@@ -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,7 +34,8 @@ 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.
@@ -41,24 +43,51 @@ export class SourceFileLoader {
* @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,15 +125,34 @@ 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);
@@ -116,7 +160,11 @@ export class SourceFileLoader {
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;
}
packages/compiler-cli/src/ngtsc/sourcemaps/test/source_file_loader_spec.ts
+138 -56
View File
File diff suppressed because it is too large Load Diff
packages/compiler-cli/src/ngtsc/sourcemaps/test/source_file_spec.ts
+198 -146
View File
File diff suppressed because it is too large Load Diff