From 44f20fc351a506af65cd04f5d49d6c269aa7817c Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 31 Oct 2020 16:07:53 -0700 Subject: [PATCH 01/10] errors: do not call resolve on URLs with schemes Based on advice from webpack project, this PR stops trying to resolve sources with a webpack:// scheme (or other schemes, as they represent absolute URLs). Source content is now loaded from the sourcesContent lookup, if it is populated (allowing for non file:// schemes). Refs: https://github.com/nodejs/node/issues/35325 --- .../source_map/prepare_stack_trace.js | 41 +++++++++++++------ lib/internal/source_map/source_map_cache.js | 6 ++- test/fixtures/source-map/webpack.js | 2 + test/fixtures/source-map/webpack.js.map | 1 + test/parallel/test-source-map-enable.js | 17 ++++++++ 5 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 test/fixtures/source-map/webpack.js create mode 100644 test/fixtures/source-map/webpack.js.map diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 23e117b677b74a..8826dff68de479 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -40,14 +40,12 @@ const prepareStackTrace = (globalThis, error, trace) => { } let errorSource = ''; - let firstSource; let firstLine; let firstColumn; const preparedTrace = trace.map((t, i) => { if (i === 0) { firstLine = t.getLineNumber(); firstColumn = t.getColumnNumber(); - firstSource = t.getFileName(); } let str = i !== 0 ? '\n at ' : ''; str = `${str}${t}`; @@ -63,16 +61,21 @@ const prepareStackTrace = (globalThis, error, trace) => { } = sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1); if (originalSource && originalLine !== undefined && originalColumn !== undefined) { - const originalSourceNoScheme = originalSource - .replace(/^file:\/\//, ''); if (i === 0) { firstLine = originalLine + 1; firstColumn = originalColumn + 1; - firstSource = originalSourceNoScheme; + // Show error in original source context to help user pinpoint it: - errorSource = getErrorSource(firstSource, firstLine, firstColumn); + errorSource = getErrorSource( + sm.payload, + originalSource, + firstLine, + firstColumn + ); } // Show both original and transpiled stack trace information: + const originalSourceNoScheme = originalSource + .replace(/^file:\/\//, ''); str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + `${originalColumn + 1}`; } @@ -88,15 +91,26 @@ const prepareStackTrace = (globalThis, error, trace) => { // Places a snippet of code from where the exception was originally thrown // above the stack trace. This logic is modeled after GetErrorSource in // node_errors.cc. -function getErrorSource(firstSource, firstLine, firstColumn) { +function getErrorSource(payload, originalSource, firstLine, firstColumn) { let exceptionLine = ''; + const originalSourceNoScheme = originalSource.replace(/^file:\/\//, ''); + let source; - try { - source = readFileSync(firstSource, 'utf8'); - } catch (err) { - debug(err); - return exceptionLine; + const sourceContentIndex = payload.sources.indexOf(originalSource); + if (payload.sourcesContent?.[sourceContentIndex]) { + // First we check if the original source content was provided in the + // source map itself: + source = payload.sourcesContent[sourceContentIndex]; + } else { + // If no sourcesContent was found, attempt to load the original source + // from disk: + try { + source = readFileSync(originalSourceNoScheme, 'utf8'); + } catch (err) { + debug(err); + } } + const lines = source.split(/\r?\n/, firstLine); const line = lines[firstLine - 1]; if (!line) return exceptionLine; @@ -110,7 +124,8 @@ function getErrorSource(firstSource, firstLine, firstColumn) { } prefix = prefix.slice(0, -1); // The last character is the '^'. - exceptionLine = `${firstSource}:${firstLine}\n${line}\n${prefix}^\n\n`; + exceptionLine = + `${originalSourceNoScheme}:${firstLine}\n${line}\n${prefix}^\n\n`; return exceptionLine; } diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index db7a19e275951c..edaa1fc63afb76 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -165,14 +165,16 @@ function sourceMapFromDataUrl(basePath, url) { // If the sources are not absolute URLs after prepending of the "sourceRoot", // the sources are resolved relative to the SourceMap (like resolving script // src in a html document). +// TODO(bcoe): refactor to use new URL(url, base), rather than path.resolve. function sourcesToAbsolute(base, data) { data.sources = data.sources.map((source) => { source = (data.sourceRoot || '') + source; + // An absolute URI does not need to be resolved, return it: + if (source.match(/(?^\w+):\/\//)) return source; if (!/^[\\/]/.test(source[0])) { source = resolve(base, source); } - if (!source.startsWith('file://')) source = `file://${source}`; - return source; + return `file://${source}`; }); // The sources array is now resolved to absolute URLs, sourceRoot should // be updated to noop. diff --git a/test/fixtures/source-map/webpack.js b/test/fixtures/source-map/webpack.js new file mode 100644 index 00000000000000..c8bf26ab6f6f59 --- /dev/null +++ b/test/fixtures/source-map/webpack.js @@ -0,0 +1,2 @@ +!function(e){var t={};function r(n){if(t[n])return t[n].exports;var o=t[n]={i:n,l:!1,exports:{}};return e[n].call(o.exports,o,o.exports,r),o.l=!0,o.exports}r.m=e,r.c=t,r.d=function(e,t,n){r.o(e,t)||Object.defineProperty(e,t,{enumerable:!0,get:n})},r.r=function(e){"undefined"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(e,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(e,"__esModule",{value:!0})},r.t=function(e,t){if(1&t&&(e=r(e)),8&t)return e;if(4&t&&"object"==typeof e&&e&&e.__esModule)return e;var n=Object.create(null);if(r.r(n),Object.defineProperty(n,"default",{enumerable:!0,value:e}),2&t&&"string"!=typeof e)for(var o in e)r.d(n,o,function(t){return e[t]}.bind(null,o));return n},r.n=function(e){var t=e&&e.__esModule?function(){return e.default}:function(){return e};return r.d(t,"a",t),t},r.o=function(e,t){return Object.prototype.hasOwnProperty.call(e,t)},r.p="",r(r.s=0)}([function(e,t){const r=()=>{n()},n=()=>{o()},o=()=>{throw new Error("oh no!")};r()}]); +//# sourceMappingURL=webpack.js.map diff --git a/test/fixtures/source-map/webpack.js.map b/test/fixtures/source-map/webpack.js.map new file mode 100644 index 00000000000000..c0849ff6f0a3ba --- /dev/null +++ b/test/fixtures/source-map/webpack.js.map @@ -0,0 +1 @@ +{"version":3,"sources":["webpack:///webpack/bootstrap","webpack:///./webpack.js"],"names":["installedModules","__webpack_require__","moduleId","exports","module","i","l","modules","call","m","c","d","name","getter","o","Object","defineProperty","enumerable","get","r","Symbol","toStringTag","value","t","mode","__esModule","ns","create","key","bind","n","object","property","prototype","hasOwnProperty","p","s","functionB","functionC","functionD","Error"],"mappings":"aACE,IAAIA,EAAmB,GAGvB,SAASC,EAAoBC,GAG5B,GAAGF,EAAiBE,GACnB,OAAOF,EAAiBE,GAAUC,QAGnC,IAAIC,EAASJ,EAAiBE,GAAY,CACzCG,EAAGH,EACHI,GAAG,EACHH,QAAS,IAUV,OANAI,EAAQL,GAAUM,KAAKJ,EAAOD,QAASC,EAAQA,EAAOD,QAASF,GAG/DG,EAAOE,GAAI,EAGJF,EAAOD,QAKfF,EAAoBQ,EAAIF,EAGxBN,EAAoBS,EAAIV,EAGxBC,EAAoBU,EAAI,SAASR,EAASS,EAAMC,GAC3CZ,EAAoBa,EAAEX,EAASS,IAClCG,OAAOC,eAAeb,EAASS,EAAM,CAAEK,YAAY,EAAMC,IAAKL,KAKhEZ,EAAoBkB,EAAI,SAAShB,GACX,oBAAXiB,QAA0BA,OAAOC,aAC1CN,OAAOC,eAAeb,EAASiB,OAAOC,YAAa,CAAEC,MAAO,WAE7DP,OAAOC,eAAeb,EAAS,aAAc,CAAEmB,OAAO,KAQvDrB,EAAoBsB,EAAI,SAASD,EAAOE,GAEvC,GADU,EAAPA,IAAUF,EAAQrB,EAAoBqB,IAC/B,EAAPE,EAAU,OAAOF,EACpB,GAAW,EAAPE,GAA8B,iBAAVF,GAAsBA,GAASA,EAAMG,WAAY,OAAOH,EAChF,IAAII,EAAKX,OAAOY,OAAO,MAGvB,GAFA1B,EAAoBkB,EAAEO,GACtBX,OAAOC,eAAeU,EAAI,UAAW,CAAET,YAAY,EAAMK,MAAOA,IACtD,EAAPE,GAA4B,iBAATF,EAAmB,IAAI,IAAIM,KAAON,EAAOrB,EAAoBU,EAAEe,EAAIE,EAAK,SAASA,GAAO,OAAON,EAAMM,IAAQC,KAAK,KAAMD,IAC9I,OAAOF,GAIRzB,EAAoB6B,EAAI,SAAS1B,GAChC,IAAIS,EAAST,GAAUA,EAAOqB,WAC7B,WAAwB,OAAOrB,EAAgB,SAC/C,WAA8B,OAAOA,GAEtC,OADAH,EAAoBU,EAAEE,EAAQ,IAAKA,GAC5BA,GAIRZ,EAAoBa,EAAI,SAASiB,EAAQC,GAAY,OAAOjB,OAAOkB,UAAUC,eAAe1B,KAAKuB,EAAQC,IAGzG/B,EAAoBkC,EAAI,GAIjBlC,EAAoBA,EAAoBmC,EAAI,G,gBClFrD,MAIMC,EAAY,KAChBC,KAGIA,EAAY,KAChBC,KAGIA,EAAY,KAChB,MAAM,IAAIC,MAAM,WAZhBH","file":"webpack.js","sourcesContent":[" \t// The module cache\n \tvar installedModules = {};\n\n \t// The require function\n \tfunction __webpack_require__(moduleId) {\n\n \t\t// Check if module is in cache\n \t\tif(installedModules[moduleId]) {\n \t\t\treturn installedModules[moduleId].exports;\n \t\t}\n \t\t// Create a new module (and put it into the cache)\n \t\tvar module = installedModules[moduleId] = {\n \t\t\ti: moduleId,\n \t\t\tl: false,\n \t\t\texports: {}\n \t\t};\n\n \t\t// Execute the module function\n \t\tmodules[moduleId].call(module.exports, module, module.exports, __webpack_require__);\n\n \t\t// Flag the module as loaded\n \t\tmodule.l = true;\n\n \t\t// Return the exports of the module\n \t\treturn module.exports;\n \t}\n\n\n \t// expose the modules object (__webpack_modules__)\n \t__webpack_require__.m = modules;\n\n \t// expose the module cache\n \t__webpack_require__.c = installedModules;\n\n \t// define getter function for harmony exports\n \t__webpack_require__.d = function(exports, name, getter) {\n \t\tif(!__webpack_require__.o(exports, name)) {\n \t\t\tObject.defineProperty(exports, name, { enumerable: true, get: getter });\n \t\t}\n \t};\n\n \t// define __esModule on exports\n \t__webpack_require__.r = function(exports) {\n \t\tif(typeof Symbol !== 'undefined' && Symbol.toStringTag) {\n \t\t\tObject.defineProperty(exports, Symbol.toStringTag, { value: 'Module' });\n \t\t}\n \t\tObject.defineProperty(exports, '__esModule', { value: true });\n \t};\n\n \t// create a fake namespace object\n \t// mode & 1: value is a module id, require it\n \t// mode & 2: merge all properties of value into the ns\n \t// mode & 4: return value when already ns object\n \t// mode & 8|1: behave like require\n \t__webpack_require__.t = function(value, mode) {\n \t\tif(mode & 1) value = __webpack_require__(value);\n \t\tif(mode & 8) return value;\n \t\tif((mode & 4) && typeof value === 'object' && value && value.__esModule) return value;\n \t\tvar ns = Object.create(null);\n \t\t__webpack_require__.r(ns);\n \t\tObject.defineProperty(ns, 'default', { enumerable: true, value: value });\n \t\tif(mode & 2 && typeof value != 'string') for(var key in value) __webpack_require__.d(ns, key, function(key) { return value[key]; }.bind(null, key));\n \t\treturn ns;\n \t};\n\n \t// getDefaultExport function for compatibility with non-harmony modules\n \t__webpack_require__.n = function(module) {\n \t\tvar getter = module && module.__esModule ?\n \t\t\tfunction getDefault() { return module['default']; } :\n \t\t\tfunction getModuleExports() { return module; };\n \t\t__webpack_require__.d(getter, 'a', getter);\n \t\treturn getter;\n \t};\n\n \t// Object.prototype.hasOwnProperty.call\n \t__webpack_require__.o = function(object, property) { return Object.prototype.hasOwnProperty.call(object, property); };\n\n \t// __webpack_public_path__\n \t__webpack_require__.p = \"\";\n\n\n \t// Load entry module and return exports\n \treturn __webpack_require__(__webpack_require__.s = 0);\n","const functionA = () => {\n functionB()\n}\n\nconst functionB = () => {\n functionC()\n}\n\nconst functionC = () => {\n functionD()\n}\n\nconst functionD = () => {\n throw new Error('oh no!')\n}\n\nfunctionA()\n"],"sourceRoot":""} \ No newline at end of file diff --git a/test/parallel/test-source-map-enable.js b/test/parallel/test-source-map-enable.js index 71130441438dcc..1b6c0bb415762d 100644 --- a/test/parallel/test-source-map-enable.js +++ b/test/parallel/test-source-map-enable.js @@ -265,6 +265,23 @@ function nextdir() { ); } +// Does not attempt to apply path resolution logic to absolute URLs +// with schemes. +// Refs: https://github.com/webpack/webpack/issues/9601 +// Refs: https://sourcemaps.info/spec.html#h.75yo6yoyk7x5 +{ + const output = spawnSync(process.execPath, [ + '--enable-source-maps', + require.resolve('../fixtures/source-map/webpack.js') + ]); + // Error in original context of source content: + assert.ok( + output.stderr.toString().match(/throw new Error\('oh no!'\)\n.*\^/) + ); + // Rewritten stack trace: + assert.ok(output.stderr.toString().includes('webpack:///./webpack.js:14:9')); +} + function getSourceMapFromCache(fixtureFile, coverageDirectory) { const jsonFiles = fs.readdirSync(coverageDirectory); for (const jsonFile of jsonFiles) { From dd0949f019c9cf36f00396bf0f4dad9c5208a649 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sun, 1 Nov 2020 00:16:34 -0400 Subject: [PATCH 02/10] Update lib/internal/source_map/source_map_cache.js Co-authored-by: devsnek --- lib/internal/source_map/source_map_cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index edaa1fc63afb76..99f8beba036a74 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -170,7 +170,7 @@ function sourcesToAbsolute(base, data) { data.sources = data.sources.map((source) => { source = (data.sourceRoot || '') + source; // An absolute URI does not need to be resolved, return it: - if (source.match(/(?^\w+):\/\//)) return source; + if (/(^\w+):/.test(source)) return source; if (!/^[\\/]/.test(source[0])) { source = resolve(base, source); } From 3e78647c4dde10121e287f4479449f5089150817 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sun, 1 Nov 2020 10:38:43 -0500 Subject: [PATCH 03/10] Update lib/internal/source_map/prepare_stack_trace.js Co-authored-by: Antoine du Hamel --- lib/internal/source_map/prepare_stack_trace.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 8826dff68de479..b1529cbe490526 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -74,8 +74,11 @@ const prepareStackTrace = (globalThis, error, trace) => { ); } // Show both original and transpiled stack trace information: - const originalSourceNoScheme = originalSource - .replace(/^file:\/\//, ''); + const originalSourceNoScheme = StringPrototypeReplace( + originalSource, + /^file:\/\//, + '' + ); str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + `${originalColumn + 1}`; } From 596cf2a5faf5e8074ce7d11ef63d3c52e1c107e1 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sun, 1 Nov 2020 10:38:50 -0500 Subject: [PATCH 04/10] Update lib/internal/source_map/prepare_stack_trace.js Co-authored-by: Antoine du Hamel --- lib/internal/source_map/prepare_stack_trace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index b1529cbe490526..c7d47cee6db5a0 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -99,7 +99,7 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) { const originalSourceNoScheme = originalSource.replace(/^file:\/\//, ''); let source; - const sourceContentIndex = payload.sources.indexOf(originalSource); + const sourceContentIndex = ArrayPrototypeIndexOf(payload.sources, originalSource); if (payload.sourcesContent?.[sourceContentIndex]) { // First we check if the original source content was provided in the // source map itself: From 8933b5ccb3d637687b9c9dbb1be9d50e8c09036f Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sun, 1 Nov 2020 10:38:58 -0500 Subject: [PATCH 05/10] Update lib/internal/source_map/source_map_cache.js Co-authored-by: Antoine du Hamel --- lib/internal/source_map/source_map_cache.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 99f8beba036a74..5681851e06e092 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -170,7 +170,7 @@ function sourcesToAbsolute(base, data) { data.sources = data.sources.map((source) => { source = (data.sourceRoot || '') + source; // An absolute URI does not need to be resolved, return it: - if (/(^\w+):/.test(source)) return source; + if (RegExpPrototypeTest(/(^\w+):/, source)) return source; if (!/^[\\/]/.test(source[0])) { source = resolve(base, source); } From 6ace772fec1d4bdde073bc441e09ecae276ad129 Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sun, 1 Nov 2020 10:39:16 -0500 Subject: [PATCH 06/10] Update lib/internal/source_map/prepare_stack_trace.js Co-authored-by: Antoine du Hamel --- lib/internal/source_map/prepare_stack_trace.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index c7d47cee6db5a0..0f95fa6bf0fe97 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -96,7 +96,8 @@ const prepareStackTrace = (globalThis, error, trace) => { // node_errors.cc. function getErrorSource(payload, originalSource, firstLine, firstColumn) { let exceptionLine = ''; - const originalSourceNoScheme = originalSource.replace(/^file:\/\//, ''); + const originalSourceNoScheme = StringPrototypeReplace(originalSource, + /^file:\/\//, ''); let source; const sourceContentIndex = ArrayPrototypeIndexOf(payload.sources, originalSource); From 21124ef242e7ad5e0c37a80f78a44a93f8f21dfe Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 1 Nov 2020 08:14:11 -0800 Subject: [PATCH 07/10] errors: use URL for resolution --- .../source_map/prepare_stack_trace.js | 16 ++++----- lib/internal/source_map/source_map_cache.js | 33 +++++++------------ test/parallel/test-source-map-enable.js | 4 +-- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index 0f95fa6bf0fe97..a9db0084e827f3 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -1,6 +1,7 @@ 'use strict'; const { + ArrayPrototypeIndexOf, Error, } = primordials; @@ -15,6 +16,7 @@ const { overrideStackTrace, maybeOverridePrepareStackTrace } = require('internal/errors'); +const { fileURLToPath } = require('url'); // Create a prettified stacktrace, inserting context from source maps // if possible. @@ -74,11 +76,8 @@ const prepareStackTrace = (globalThis, error, trace) => { ); } // Show both original and transpiled stack trace information: - const originalSourceNoScheme = StringPrototypeReplace( - originalSource, - /^file:\/\//, - '' - ); + const originalSourceNoScheme = originalSource.match(/^file:\/\//) ? + fileURLToPath(originalSource) : originalSource; str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + `${originalColumn + 1}`; } @@ -96,11 +95,12 @@ const prepareStackTrace = (globalThis, error, trace) => { // node_errors.cc. function getErrorSource(payload, originalSource, firstLine, firstColumn) { let exceptionLine = ''; - const originalSourceNoScheme = StringPrototypeReplace(originalSource, - /^file:\/\//, ''); + const originalSourceNoScheme = originalSource.match(/^file:\/\//) ? + fileURLToPath(originalSource) : originalSource; let source; - const sourceContentIndex = ArrayPrototypeIndexOf(payload.sources, originalSource); + const sourceContentIndex = + ArrayPrototypeIndexOf(payload.sources, originalSource); if (payload.sourcesContent?.[sourceContentIndex]) { // First we check if the original source content was provided in the // source map itself: diff --git a/lib/internal/source_map/source_map_cache.js b/lib/internal/source_map/source_map_cache.js index 5681851e06e092..f94ffa8387a451 100644 --- a/lib/internal/source_map/source_map_cache.js +++ b/lib/internal/source_map/source_map_cache.js @@ -25,7 +25,6 @@ const { Buffer } = require('buffer'); let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => { debug = fn; }); -const { dirname, resolve } = require('path'); const fs = require('fs'); const { getOptionValue } = require('internal/options'); const { @@ -63,10 +62,8 @@ function getSourceMapsEnabled() { function maybeCacheSourceMap(filename, content, cjsModuleInstance) { const sourceMapsEnabled = getSourceMapsEnabled(); if (!(process.env.NODE_V8_COVERAGE || sourceMapsEnabled)) return; - let basePath; try { filename = normalizeReferrerURL(filename); - basePath = dirname(fileURLToPath(filename)); } catch (err) { // This is most likely an [eval]-wrapper, which is currently not // supported. @@ -76,7 +73,7 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) { const match = content.match(/\/[*/]#\s+sourceMappingURL=(?[^\s]+)/); if (match) { - const data = dataFromUrl(basePath, match.groups.sourceMappingURL); + const data = dataFromUrl(filename, match.groups.sourceMappingURL); const url = data ? null : match.groups.sourceMappingURL; if (cjsModuleInstance) { if (!Module) Module = require('internal/modules/cjs/loader').Module; @@ -98,12 +95,12 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) { } } -function dataFromUrl(basePath, sourceMappingURL) { +function dataFromUrl(sourceURL, sourceMappingURL) { try { const url = new URL(sourceMappingURL); switch (url.protocol) { case 'data:': - return sourceMapFromDataUrl(basePath, url.pathname); + return sourceMapFromDataUrl(sourceURL, url.pathname); default: debug(`unknown protocol ${url.protocol}`); return null; @@ -111,8 +108,8 @@ function dataFromUrl(basePath, sourceMappingURL) { } catch (err) { debug(err.stack); // If no scheme is present, we assume we are dealing with a file path. - const sourceMapFile = resolve(basePath, sourceMappingURL); - return sourceMapFromFile(sourceMapFile); + const mapURL = new URL(sourceMappingURL, sourceURL).href; + return sourceMapFromFile(mapURL); } } @@ -128,11 +125,11 @@ function lineLengths(content) { }); } -function sourceMapFromFile(sourceMapFile) { +function sourceMapFromFile(mapURL) { try { - const content = fs.readFileSync(sourceMapFile, 'utf8'); + const content = fs.readFileSync(fileURLToPath(mapURL), 'utf8'); const data = JSONParse(content); - return sourcesToAbsolute(dirname(sourceMapFile), data); + return sourcesToAbsolute(mapURL, data); } catch (err) { debug(err.stack); return null; @@ -141,7 +138,7 @@ function sourceMapFromFile(sourceMapFile) { // data:[][;base64], see: // https://tools.ietf.org/html/rfc2397#section-2 -function sourceMapFromDataUrl(basePath, url) { +function sourceMapFromDataUrl(sourceURL, url) { const [format, data] = url.split(','); const splitFormat = format.split(';'); const contentType = splitFormat[0]; @@ -151,7 +148,7 @@ function sourceMapFromDataUrl(basePath, url) { Buffer.from(data, 'base64').toString('utf8') : data; try { const parsedData = JSONParse(decodedData); - return sourcesToAbsolute(basePath, parsedData); + return sourcesToAbsolute(sourceURL, parsedData); } catch (err) { debug(err.stack); return null; @@ -165,16 +162,10 @@ function sourceMapFromDataUrl(basePath, url) { // If the sources are not absolute URLs after prepending of the "sourceRoot", // the sources are resolved relative to the SourceMap (like resolving script // src in a html document). -// TODO(bcoe): refactor to use new URL(url, base), rather than path.resolve. -function sourcesToAbsolute(base, data) { +function sourcesToAbsolute(baseURL, data) { data.sources = data.sources.map((source) => { source = (data.sourceRoot || '') + source; - // An absolute URI does not need to be resolved, return it: - if (RegExpPrototypeTest(/(^\w+):/, source)) return source; - if (!/^[\\/]/.test(source[0])) { - source = resolve(base, source); - } - return `file://${source}`; + return new URL(source, baseURL).href; }); // The sources array is now resolved to absolute URLs, sourceRoot should // be updated to noop. diff --git a/test/parallel/test-source-map-enable.js b/test/parallel/test-source-map-enable.js index 1b6c0bb415762d..438cc0933a8e87 100644 --- a/test/parallel/test-source-map-enable.js +++ b/test/parallel/test-source-map-enable.js @@ -276,10 +276,10 @@ function nextdir() { ]); // Error in original context of source content: assert.ok( - output.stderr.toString().match(/throw new Error\('oh no!'\)\n.*\^/) + output.stderr.toString().match(/throw new Error\('oh no!'\)\r?\n.*\^/) ); // Rewritten stack trace: - assert.ok(output.stderr.toString().includes('webpack:///./webpack.js:14:9')); + assert.ok(output.stderr.toString().includes('webpack:///webpack.js:14:9')); } function getSourceMapFromCache(fixtureFile, coverageDirectory) { From 3c1a7cad76fcb91014934ba4118a5ba8e6d7555f Mon Sep 17 00:00:00 2001 From: "Benjamin E. Coe" Date: Sun, 1 Nov 2020 14:29:10 -0500 Subject: [PATCH 08/10] chore: address code review Co-authored-by: Antoine du Hamel --- lib/internal/source_map/prepare_stack_trace.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index a9db0084e827f3..ea8bd2a2a8aa0c 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -16,7 +16,7 @@ const { overrideStackTrace, maybeOverridePrepareStackTrace } = require('internal/errors'); -const { fileURLToPath } = require('url'); +const { fileURLToPath } = require('internal/url'); // Create a prettified stacktrace, inserting context from source maps // if possible. @@ -76,8 +76,9 @@ const prepareStackTrace = (globalThis, error, trace) => { ); } // Show both original and transpiled stack trace information: - const originalSourceNoScheme = originalSource.match(/^file:\/\//) ? - fileURLToPath(originalSource) : originalSource; + const originalSourceNoScheme = + StringPrototypeStarsWith(originalSource, 'file://') ? + fileURLToPath(originalSource) : originalSource; str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + `${originalColumn + 1}`; } @@ -95,8 +96,9 @@ const prepareStackTrace = (globalThis, error, trace) => { // node_errors.cc. function getErrorSource(payload, originalSource, firstLine, firstColumn) { let exceptionLine = ''; - const originalSourceNoScheme = originalSource.match(/^file:\/\//) ? - fileURLToPath(originalSource) : originalSource; + const originalSourceNoScheme = + StringPrototypeStartsWith(originalSource, 'file://') ? + fileURLToPath(originalSource) : originalSource; let source; const sourceContentIndex = From eafd35e351329b92f251466d2b5aa40efc725f43 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 1 Nov 2020 13:00:58 -0800 Subject: [PATCH 09/10] chore: fix linting --- lib/internal/source_map/prepare_stack_trace.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index ea8bd2a2a8aa0c..e9c3536c963504 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -3,6 +3,7 @@ const { ArrayPrototypeIndexOf, Error, + StringPrototypeStartsWith, } = primordials; let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => { @@ -77,7 +78,7 @@ const prepareStackTrace = (globalThis, error, trace) => { } // Show both original and transpiled stack trace information: const originalSourceNoScheme = - StringPrototypeStarsWith(originalSource, 'file://') ? + StringPrototypeStartsWith(originalSource, 'file://') ? fileURLToPath(originalSource) : originalSource; str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + `${originalColumn + 1}`; From c85b48035feb29edf11f02aaa9eb18964ca1eb31 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 1 Nov 2020 15:11:53 -0800 Subject: [PATCH 10/10] chore: fix path resolution --- test/parallel/test-source-map-enable.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-source-map-enable.js b/test/parallel/test-source-map-enable.js index 438cc0933a8e87..0887ae8811c45b 100644 --- a/test/parallel/test-source-map-enable.js +++ b/test/parallel/test-source-map-enable.js @@ -8,6 +8,7 @@ const { dirname } = require('path'); const fs = require('fs'); const path = require('path'); const { spawnSync } = require('child_process'); +const { pathToFileURL } = require('url'); const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -88,8 +89,8 @@ function nextdir() { // Source-map should have been loaded from disk and sources should have been // rewritten, such that they're absolute paths. assert.strictEqual( - dirname( - `file://${require.resolve('../fixtures/source-map/disk-relative-path')}`), + dirname(pathToFileURL( + require.resolve('../fixtures/source-map/disk-relative-path')).href), dirname(sourceMap.data.sources[0]) ); } @@ -109,8 +110,8 @@ function nextdir() { // base64 JSON should have been decoded, and paths to sources should have // been rewritten such that they're absolute: assert.strictEqual( - dirname( - `file://${require.resolve('../fixtures/source-map/inline-base64')}`), + dirname(pathToFileURL( + require.resolve('../fixtures/source-map/inline-base64')).href), dirname(sourceMap.data.sources[0]) ); }