Skip to content

lib: don't parse windows drive letters as schemes #50580

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.

Sign up for GitHub

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

Merged
nodejs-github-bot merged 3 commits into from
Dec 5, 2023
Merged

lib: don't parse windows drive letters as schemes #50580

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
64365ad
lib: don't parse windows drive letters as schemes
PaperStrike Nov 6, 2023
3daf3b8
lib: typo fixes
PaperStrike Nov 25, 2023
f41d758
lib: update sourcesToAbsolute comment
PaperStrike Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
lib: don't parse windows drive letters as schemes 
We were incorrectly parsing windows drive letters as schemes. This was
polluting the source map cache with malformed file paths.

Fixes: #50523
  • Loading branch information
@PaperStrike
PaperStrike committed Nov 6, 2023
commit 64365adb1ce5628bf535001032e2480e52f5a15a
4 changes: 4 additions & 0 deletions lib/internal/source_map/source_map_cache.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const kLeadingProtocol = /^\w+:\/\//;
const kSourceMappingURLMagicComment = /\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/g;
const kSourceURLMagicComment = /\/[*/]#\s+sourceURL=(?<sourceURL>[^\s]+)/g;

const { isAbsolute } = require('path');
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');

let SourceMap;
Expand Down Expand Up @@ -254,6 +255,9 @@ function sourceMapFromDataUrl(sourceURL, url) {
function sourcesToAbsolute(baseURL, data) {
data.sources = data.sources.map((source) => {
source = (data.sourceRoot || '') + source;
if (isAbsolute(source)) {
Comment thread
PaperStrike marked this conversation as resolved.
return pathToFileURL(source).href;
}
return new URL(source, baseURL).href;
});
// The sources array is now resolved to absolute URLs, sourceRoot should
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/source-map/ts-node-win32.js
View file
Open in desktop

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

11 changes: 11 additions & 0 deletions test/fixtures/source-map/ts-node.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
function foo(str: string) {
return str;
}

foo('noop');

// To recreate (only on windows):
//
// const filePath = require.resolve('./test/fixtures/source-map/ts-node.ts');
// const compiled = require('ts-node').create({ transpileOnly: true }).compile(fs.readFileSync(filePath, 'utf8'), filePath);
// fs.writeFileSync('test/fixtures/source-map/ts-node-win32.js', compiled);
24 changes: 24 additions & 0 deletions test/parallel/test-source-map-enable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,30 @@ function nextdir() {
assert.match(output.stderr.toString(), /at functionC.*10:3/);
}

// Properly converts windows absolute paths to absolute URLs
Comment thread
PaperStrike marked this conversation as resolved.
Outdated
// Refs: https://github.com/nodejs/node/issues/50523
// Refs: https://github.com/TypeStrong/ts-node/issues/1769
{
const coverageDirectory = nextdir();
const output = spawnSync(process.execPath, [
require.resolve('../fixtures/source-map/ts-node-win32.js'),
], { env: { ...process.env, NODE_V8_COVERAGE: coverageDirectory } });
assert.strictEqual(output.status, 0);
assert.strictEqual(output.stderr.toString(), '');
const sourceMap = getSourceMapFromCache(
'ts-node-win32.js',
coverageDirectory
);
// base64 JSON should have been decoded, the D: in the sources field should
// have been taken as the drive letter on Windows, the scheme on POSIX.
assert.strictEqual(
sourceMap.data.sources[0],
common.isWindows ?
'file:///D:/workspaces/node/test/fixtures/source-map/ts-node.ts' :
'd:/workspaces/node/test/fixtures/source-map/ts-node.ts'
);
}

// Stores and applies source map associated with file that throws while
// being required.
{
Expand Down