Normalize consumer URLs to be as absolute as possible to avoid usage of 'util.relative'.

Author: loganfsmyth

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,36 +194,16 @@ 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
       // #72 and bugzil.la/889492.
       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.');
   }
 
   /**

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();
 };