Skip to content

Commit

Permalink
module: port source map sorting logic from chromium
Browse files Browse the repository at this point in the history
Digging in to the delta between V8's source map library, and chromium's
the most significant difference that jumped out at me was that we were
failing to sort generated columns. Since negative offsets are not
restricted in the spec, this can lead to bugs.

fixes: #31286
  • Loading branch information
bcoe committed Feb 24, 2020
1 parent 9c70292 commit 584bdd8
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
27 changes: 26 additions & 1 deletion lib/internal/source_map/source_map.js
Expand Up @@ -212,6 +212,7 @@ class SourceMap {
let sourceIndex = 0;
let sourceLineNumber = 0;
let sourceColumnNumber = 0;
let hadNegativeColumnOffset = false;

const sources = [];
const originalToCanonicalURLMap = {};
Expand Down Expand Up @@ -241,7 +242,13 @@ class SourceMap {
break;
}

columnNumber += decodeVLQ(stringCharIterator);
// A negative columnOffset is valid, and if one is observed sorting is
// necessary, see: https://github.com/mozilla/source-map/pull/92
const columnOffset = decodeVLQ(stringCharIterator);
if (columnOffset < 0) {
hadNegativeColumnOffset = true;
}
columnNumber += columnOffset;
if (isSeparator(stringCharIterator.peek())) {
this.#mappings.push([lineNumber, columnNumber]);
continue;
Expand All @@ -261,6 +268,9 @@ class SourceMap {
this.#mappings.push([lineNumber, columnNumber, sourceURL,
sourceLineNumber, sourceColumnNumber]);
}
if (hadNegativeColumnOffset) {
this.#mappings.sort(compareSourceMapEntry);
}
};
}

Expand Down Expand Up @@ -321,6 +331,21 @@ function cloneSourceMapV3(payload) {
return payload;
}

/**
* @param {Array} entry1 source map entry [lineNumber, columnNumber, sourceURL,
* sourceLineNumber, sourceColumnNumber]
* @param {Array} entry2 source map entry.
* @return {number}
*/
function compareSourceMapEntry(entry1, entry2) {
const [lineNumber1, columnNumber1] = entry1;
const [lineNumber2, columnNumber2] = entry2;
if (lineNumber1 !== lineNumber2) {
return lineNumber1 - lineNumber2;
}
return columnNumber1 - columnNumber2;
}

module.exports = {
SourceMap
};
25 changes: 25 additions & 0 deletions test/parallel/test-source-map-api.js
Expand Up @@ -124,3 +124,28 @@ const { readFileSync } = require('fs');
assert.strictEqual(originalColumn, knownDecodings[column]);
}
}

// Test that generated columns are sorted when a negative offset is
// observed, see: https://github.com/mozilla/source-map/pull/92
{
function makeMinimalMap(generatedColumns, originalColumns) {
return {
sources: ['test.js'],
// Mapping from the 0th line, 0th column of the output file to the 0th
// source file, 0th line, ${column}th column.
mappings: generatedColumns.map((g, i) => `${g}AA${originalColumns[i]}`)
.join(',')
};
}
// U = 10
// F = -2
// A = 0
// E = 2
const sourceMap = new SourceMap(makeMinimalMap(
['U', 'F', 'F'],
['A', 'E', 'E']
));
assert.strictEqual(sourceMap.findEntry(0, 6).originalColumn, 4);
assert.strictEqual(sourceMap.findEntry(0, 8).originalColumn, 2);
assert.strictEqual(sourceMap.findEntry(0, 10).originalColumn, 0);
}

0 comments on commit 584bdd8

Please sign in to comment.