From f91f1407898a8c5da0ff4aa535cc11649fa54e55 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 31 Oct 2020 16:07:53 -0700 Subject: [PATCH] errors: do not call resolve on URLs with schemes We were incorrectly trying to run path.resolve on absolute sources URLs. This was breaking webpack:// URLs in stack trace output. Refs: https://github.com/nodejs/node/issues/35325 Backport-PR-URL: https://github.com/nodejs/node/pull/37717 PR-URL: https://github.com/nodejs/node/pull/35903 Reviewed-By: Rich Trott Reviewed-By: Antoine du Hamel Reviewed-By: Matteo Collina --- .../source_map/prepare_stack_trace.js | 48 ++++++++++++++----- lib/internal/source_map/source_map_cache.js | 31 +++++------- test/fixtures/source-map/webpack.js | 2 + test/fixtures/source-map/webpack.js.map | 1 + test/parallel/test-source-map-enable.js | 26 ++++++++-- 5 files changed, 72 insertions(+), 36 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..e9c3536c963504 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -1,7 +1,9 @@ 'use strict'; const { + ArrayPrototypeIndexOf, Error, + StringPrototypeStartsWith, } = primordials; let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => { @@ -15,6 +17,7 @@ const { overrideStackTrace, maybeOverridePrepareStackTrace } = require('internal/errors'); +const { fileURLToPath } = require('internal/url'); // Create a prettified stacktrace, inserting context from source maps // if possible. @@ -40,14 +43,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 +64,22 @@ 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 = + StringPrototypeStartsWith(originalSource, 'file://') ? + fileURLToPath(originalSource) : originalSource; str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + `${originalColumn + 1}`; } @@ -88,15 +95,29 @@ 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 = + StringPrototypeStartsWith(originalSource, 'file://') ? + fileURLToPath(originalSource) : originalSource; + let source; - try { - source = readFileSync(firstSource, 'utf8'); - } catch (err) { - debug(err); - return exceptionLine; + 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: + 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 +131,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..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,14 +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). -function sourcesToAbsolute(base, data) { +function sourcesToAbsolute(baseURL, data) { data.sources = data.sources.map((source) => { source = (data.sourceRoot || '') + source; - if (!/^[\\/]/.test(source[0])) { - source = resolve(base, source); - } - if (!source.startsWith('file://')) source = `file://${source}`; - return 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/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..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]) ); } @@ -265,6 +266,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!'\)\r?\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) {