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

errors: do not call resolve on URLs with schemes #35903

Closed
wants to merge 10 commits into from
45 changes: 32 additions & 13 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const {
ArrayPrototypeIndexOf,
Error,
} = primordials;

Expand All @@ -15,6 +16,7 @@ const {
overrideStackTrace,
maybeOverridePrepareStackTrace
} = require('internal/errors');
const { fileURLToPath } = require('url');
bcoe marked this conversation as resolved.
Show resolved Hide resolved

// Create a prettified stacktrace, inserting context from source maps
// if possible.
Expand All @@ -40,14 +42,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 +63,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.match(/^file:\/\//) ?
fileURLToPath(originalSource) : originalSource;
bcoe marked this conversation as resolved.
Show resolved Hide resolved
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
`${originalColumn + 1}`;
}
Expand All @@ -88,15 +93,28 @@ 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.match(/^file:\/\//) ?
fileURLToPath(originalSource) : originalSource;
bcoe marked this conversation as resolved.
Show resolved Hide resolved

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 +128,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
Original file line number Diff line number Diff line change
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.

17 changes: 17 additions & 0 deletions test/parallel/test-source-map-enable.js
Original file line number Diff line number Diff line change
Expand Up @@ -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!'\)\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