From fabe6587359568b9717d704578ca84754a5365d3 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Thu, 11 Oct 2018 09:10:46 -0700 Subject: [PATCH] Normalize consumer URLs to be as absolute as possible to avoid usage of 'util.relative'. --- lib/source-map-consumer.js | 84 ++++++-------------------------- test/test-source-map-consumer.js | 7 +-- 2 files changed, 20 insertions(+), 71 deletions(-) diff --git a/lib/source-map-consumer.js b/lib/source-map-consumer.js index 5e36c99c..397b2b10 100644 --- a/lib/source-map-consumer.js +++ b/lib/source-map-consumer.js @@ -179,11 +179,11 @@ class BasicSourceMapConsumer extends SourceMapConsumer { } const version = util.getArg(sourceMap, "version"); - let sources = util.getArg(sourceMap, "sources"); + const sources = util.getArg(sourceMap, "sources").map(String); // Sass 3.3 leaves out the 'names' array, so we deviate from the spec (which // requires the array) to play nice here. const names = util.getArg(sourceMap, "names", []); - let sourceRoot = util.getArg(sourceMap, "sourceRoot", null); + const sourceRoot = util.getArg(sourceMap, "sourceRoot", null); const sourcesContent = util.getArg(sourceMap, "sourcesContent", null); const mappings = util.getArg(sourceMap, "mappings"); const file = util.getArg(sourceMap, "file", null); @@ -194,26 +194,6 @@ class BasicSourceMapConsumer extends SourceMapConsumer { throw new Error("Unsupported version: " + version); } - if (sourceRoot) { - sourceRoot = util.normalize(sourceRoot); - } - - sources = sources - .map(String) - // Some source maps produce relative source paths like "./foo.js" instead of - // "foo.js". Normalize these first so that future comparisons will succeed. - // See bugzil.la/1090768. - .map(util.normalize) - // Always ensure that absolute sources are internally stored relative to - // the source root, if the source root is absolute. Not doing this would - // be particularly problematic when the source root is a prefix of the - // source (valid, but why??). See github issue #199 and bugzil.la/1188982. - .map(function(source) { - return sourceRoot && util.isAbsolute(sourceRoot) && util.isAbsolute(source) - ? util.relative(sourceRoot, source) - : source; - }); - // Pass `true` below to allow duplicate names and sources. While source maps // are intended to be compressed and deduplicated, the TypeScript compiler // sometimes generates source maps with duplicates in them. See Github issue @@ -221,9 +201,9 @@ class BasicSourceMapConsumer extends SourceMapConsumer { that._names = ArraySet.fromArray(names.map(String), true); that._sources = ArraySet.fromArray(sources, true); - that._absoluteSources = that._sources.toArray().map(function(s) { + that._absoluteSources = ArraySet.fromArray(that._sources.toArray().map(function(s) { return util.computeSourceURL(sourceRoot, s, aSourceMapURL); - }); + }), true); that.sourceRoot = sourceRoot; that.sourcesContent = sourcesContent; @@ -247,21 +227,16 @@ class BasicSourceMapConsumer extends SourceMapConsumer { * found. */ _findSourceIndex(aSource) { - let relativeSource = aSource; - if (this.sourceRoot != null) { - relativeSource = util.relative(this.sourceRoot, relativeSource); + // Treat the source as map-relative overall by default. + const sourceAsMapRelative = util.computeSourceURL(null, aSource, this._sourceMapURL); + if (this._absoluteSources.has(sourceAsMapRelative)) { + return this._absoluteSources.indexOf(sourceAsMapRelative); } - if (this._sources.has(relativeSource)) { - return this._sources.indexOf(relativeSource); - } - - // Maybe aSource is an absolute URL as returned by |sources|. In - // this case we can't simply undo the transform. - for (let i = 0; i < this._absoluteSources.length; ++i) { - if (this._absoluteSources[i] == aSource) { - return i; - } + // Fall back to treating the source as sourceRoot-relative. + const sourceAsSourceRootRelative = util.computeSourceURL(this.sourceRoot, aSource, this._sourceMapURL); + if (this._absoluteSources.has(sourceAsSourceRootRelative)) { + return this._absoluteSources.indexOf(sourceAsSourceRootRelative); } return -1; @@ -281,7 +256,7 @@ class BasicSourceMapConsumer extends SourceMapConsumer { } get sources() { - return this._absoluteSources.slice(); + return this._absoluteSources.toArray(); } _getMappingsPtr() { @@ -341,13 +316,11 @@ class BasicSourceMapConsumer extends SourceMapConsumer { eachMapping(aCallback, aContext, aOrder) { const context = aContext || null; const order = aOrder || SourceMapConsumer.GENERATED_ORDER; - const sourceRoot = this.sourceRoot; this._wasm.withMappingCallback( mapping => { if (mapping.source !== null) { - mapping.source = this._sources.at(mapping.source); - mapping.source = util.computeSourceURL(sourceRoot, mapping.source, this._sourceMapURL); + mapping.source = this._absoluteSources.at(mapping.source); if (mapping.name !== null) { mapping.name = this._names.at(mapping.name); @@ -496,8 +469,7 @@ class BasicSourceMapConsumer extends SourceMapConsumer { if (mapping.generatedLine === needle.generatedLine) { let source = util.getArg(mapping, "source", null); if (source !== null) { - source = this._sources.at(source); - source = util.computeSourceURL(this.sourceRoot, source, this._sourceMapURL); + source = this._absoluteSources.at(source); } let name = util.getArg(mapping, "name", null); @@ -549,30 +521,6 @@ class BasicSourceMapConsumer extends SourceMapConsumer { return this.sourcesContent[index]; } - let relativeSource = aSource; - if (this.sourceRoot != null) { - relativeSource = util.relative(this.sourceRoot, relativeSource); - } - - let url; - if (this.sourceRoot != null - && (url = util.urlParse(this.sourceRoot))) { - // XXX: file:// URIs and absolute paths lead to unexpected behavior for - // many users. We can help them out when they expect file:// URIs to - // behave like it would if they were running a local HTTP server. See - // https://bugzilla.mozilla.org/show_bug.cgi?id=885597. - const fileUriAbsPath = relativeSource.replace(/^file:\/\//, ""); - if (url.scheme == "file" - && this._sources.has(fileUriAbsPath)) { - return this.sourcesContent[this._sources.indexOf(fileUriAbsPath)]; - } - - if ((!url.path || url.path == "/") - && this._sources.has("/" + relativeSource)) { - return this.sourcesContent[this._sources.indexOf("/" + relativeSource)]; - } - } - // This function is used recursively from // IndexedSourceMapConsumer.prototype.sourceContentFor. In that case, we // don't want to throw if we can't find the source - we just want to @@ -581,7 +529,7 @@ class BasicSourceMapConsumer extends SourceMapConsumer { return null; } - throw new Error('"' + relativeSource + '" is not in the SourceMap.'); + throw new Error('"' + aSource + '" is not in the SourceMap.'); } /** diff --git a/test/test-source-map-consumer.js b/test/test-source-map-consumer.js index e715d641..d2d51a09 100644 --- a/test/test-source-map-consumer.js +++ b/test/test-source-map-consumer.js @@ -1424,9 +1424,10 @@ exports["test non-normalized sourceRoot (from issue #227)"] = async function(ass sourceRoot: "./src/", sourcesContent: [ 'var name = "Mark"\n' ] }); - assert.equal(consumer.sourceRoot, "src/", "sourceRoot was normalized"); - // Before the fix, this threw an exception. - consumer.sourceContentFor(consumer.sources[0]); + assert.doesNotThrow(() => { + // Before the fix, this threw an exception. + consumer.sourceContentFor(consumer.sources[0]); + }); consumer.destroy(); };