Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize all URLs to be as absolute as possible to avoid usage of 'util.relative'. #365

Closed
wants to merge 1 commit into from
Closed

Normalize all URLs to be as absolute as possible to avoid usage of 'util.relative'. #365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 16 additions & 68 deletions lib/source-map-consumer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -281,7 +256,7 @@ class BasicSourceMapConsumer extends SourceMapConsumer {
}

get sources() {
return this._absoluteSources.slice();
return this._absoluteSources.toArray();
}

_getMappingsPtr() {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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.');
}

/**
Expand Down
7 changes: 4 additions & 3 deletions test/test-source-map-consumer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
Expand Down