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
41 changes: 28 additions & 13 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand All @@ -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:\/\//, '');
bcoe marked this conversation as resolved.
Show resolved Hide resolved
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
`${originalColumn + 1}`;
}
Expand All @@ -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:\/\//, '');
bcoe marked this conversation as resolved.
Show resolved Hide resolved
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 = payload.sources.indexOf(originalSource);
bcoe marked this conversation as resolved.
Show resolved Hide resolved
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 +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;
}

Expand Down
6 changes: 4 additions & 2 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (/(^\w+):/.test(source)) return source;
bcoe marked this conversation as resolved.
Show resolved Hide resolved
bcoe marked this conversation as resolved.
Show resolved Hide resolved
if (!/^[\\/]/.test(source[0])) {
source = resolve(base, source);
}
if (!source.startsWith('file://')) source = `file://${source}`;
return source;
return `file://${source}`;
bcoe marked this conversation as resolved.
Show resolved Hide resolved
});
// 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!'\)\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