From 584bdd8af85c505a865d98ae20893256281ba252 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 23 Feb 2020 19:19:15 -0800 Subject: [PATCH 1/2] module: port source map sorting logic from chromium 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 --- lib/internal/source_map/source_map.js | 27 ++++++++++++++++++++++++++- test/parallel/test-source-map-api.js | 25 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/lib/internal/source_map/source_map.js b/lib/internal/source_map/source_map.js index c440dffdf81913..2083993f23325d 100644 --- a/lib/internal/source_map/source_map.js +++ b/lib/internal/source_map/source_map.js @@ -212,6 +212,7 @@ class SourceMap { let sourceIndex = 0; let sourceLineNumber = 0; let sourceColumnNumber = 0; + let hadNegativeColumnOffset = false; const sources = []; const originalToCanonicalURLMap = {}; @@ -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; @@ -261,6 +268,9 @@ class SourceMap { this.#mappings.push([lineNumber, columnNumber, sourceURL, sourceLineNumber, sourceColumnNumber]); } + if (hadNegativeColumnOffset) { + this.#mappings.sort(compareSourceMapEntry); + } }; } @@ -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 }; diff --git a/test/parallel/test-source-map-api.js b/test/parallel/test-source-map-api.js index 2bfbc08809e9a1..1a61a27a63dc50 100644 --- a/test/parallel/test-source-map-api.js +++ b/test/parallel/test-source-map-api.js @@ -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); +} From 3e6b697d4264b6eb087dced2efb8296bff9a4498 Mon Sep 17 00:00:00 2001 From: bcoe Date: Wed, 26 Feb 2020 23:11:06 -0800 Subject: [PATCH 2/2] chore: address code review --- lib/internal/source_map/source_map.js | 18 +++++------------- test/parallel/test-source-map-api.js | 2 +- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/lib/internal/source_map/source_map.js b/lib/internal/source_map/source_map.js index 2083993f23325d..acff068be2a6e7 100644 --- a/lib/internal/source_map/source_map.js +++ b/lib/internal/source_map/source_map.js @@ -152,10 +152,12 @@ class SourceMap { * @param {SourceMapV3} mappingPayload */ #parseMappingPayload = () => { - if (this.#payload.sections) + if (this.#payload.sections) { this.#parseSections(this.#payload.sections); - else + } else { this.#parseMap(this.#payload, 0, 0); + } + this.#mappings.sort(compareSourceMapEntry); } /** @@ -212,7 +214,6 @@ class SourceMap { let sourceIndex = 0; let sourceLineNumber = 0; let sourceColumnNumber = 0; - let hadNegativeColumnOffset = false; const sources = []; const originalToCanonicalURLMap = {}; @@ -242,13 +243,7 @@ class SourceMap { break; } - // 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; + columnNumber += decodeVLQ(stringCharIterator); if (isSeparator(stringCharIterator.peek())) { this.#mappings.push([lineNumber, columnNumber]); continue; @@ -268,9 +263,6 @@ class SourceMap { this.#mappings.push([lineNumber, columnNumber, sourceURL, sourceLineNumber, sourceColumnNumber]); } - if (hadNegativeColumnOffset) { - this.#mappings.sort(compareSourceMapEntry); - } }; } diff --git a/test/parallel/test-source-map-api.js b/test/parallel/test-source-map-api.js index 1a61a27a63dc50..60bbb661e1c801 100644 --- a/test/parallel/test-source-map-api.js +++ b/test/parallel/test-source-map-api.js @@ -131,7 +131,7 @@ const { readFileSync } = require('fs'); function makeMinimalMap(generatedColumns, originalColumns) { return { sources: ['test.js'], - // Mapping from the 0th line, 0th column of the output file to the 0th + // Mapping from the 0th line, ${g}th 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(',')