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.

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

[v14.x backport] errors: do not call resolve on URLs with schemes #37717

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 35 additions & 13 deletions 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) => {
Expand All @@ -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.
Expand All @@ -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}`;
Expand All @@ -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}`;
}
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
31 changes: 12 additions & 19 deletions lib/internal/source_map/source_map_cache.js
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -76,7 +73,7 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {

const match = content.match(/\/[*/]#\s+sourceMappingURL=(?<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;
Expand All @@ -98,21 +95,21 @@ 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;
}
} 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);
}
}

Expand All @@ -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;
Expand All @@ -141,7 +138,7 @@ function sourceMapFromFile(sourceMapFile) {

// data:[<mediatype>][;base64],<data> 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];
Expand All @@ -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;
Expand All @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/source-map/webpack.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/fixtures/source-map/webpack.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 22 additions & 4 deletions test/parallel/test-source-map-enable.js
Expand Up @@ -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();
Expand Down Expand Up @@ -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])
);
}
Expand All @@ -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])
);
}
Expand Down Expand Up @@ -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) {
Expand Down