Repository navigation
module,win: fix long path resolve #53294
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| #include "node_errors.h" | ||
| #include "node_external_reference.h" | ||
| #include "node_url.h" | ||
| #include "path.h" | ||
| #include "permission/permission.h" | ||
| #include "permission/permission_base.h" | ||
| #include "util-inl.h" | ||
|
|
@@ -241,7 +242,7 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) { | |
| Realm* realm = Realm::GetCurrent(args); | ||
| auto isolate = realm->isolate(); | ||
|
|
||
| Utf8Value path(isolate, args[0]); | ||
| BufferValue path(isolate, args[0]); | ||
| bool is_esm = args[1]->IsTrue(); | ||
| auto error_context = ErrorContext(); | ||
| if (is_esm) { | ||
|
|
@@ -261,15 +262,10 @@ void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) { | |
| permission::PermissionScope::kFileSystemRead, | ||
| path.ToStringView()); | ||
|
|
||
| // TODO(StefanStojanovic): Remove ifdef after | ||
| // path.toNamespacedPath logic is ported to C++ | ||
| #ifdef _WIN32 | ||
| auto package_json = GetPackageJSON( | ||
| realm, "\\\\?\\" + path.ToString(), is_esm ? &error_context : nullptr); | ||
| #else | ||
| ToNamespacedPath(realm->env(), &path); | ||
| auto package_json = | ||
| GetPackageJSON(realm, path.ToString(), is_esm ? &error_context : nullptr); | ||
| #endif | ||
|
|
||
| if (package_json == nullptr) { | ||
| return; | ||
| } | ||
|
|
@@ -323,9 +319,22 @@ void BindingData::GetNearestParentPackageJSON( | |
| CHECK(args[0]->IsString()); | ||
|
|
||
| Realm* realm = Realm::GetCurrent(args); | ||
| Utf8Value path_value(realm->isolate(), args[0]); | ||
| BufferValue path_value(realm->isolate(), args[0]); | ||
| // Check if the path has a trailing slash. If so, add it after | ||
| // ToNamespacedPath() as it will be deleted by ToNamespacedPath() | ||
| bool slashCheck = !path_value.ToString().empty() && | ||
| path_value.ToStringView().ends_with( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous line is not needed Can you also remove this in other places?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| std::filesystem::path::preferred_separator); | ||
|
|
||
| ToNamespacedPath(realm->env(), &path_value); | ||
|
|
||
| std::string path_value_str = path_value.ToString(); | ||
| if (slashCheck) { | ||
| path_value_str.push_back(std::filesystem::path::preferred_separator); | ||
| } | ||
|
|
||
| auto package_json = | ||
| TraverseParent(realm, std::filesystem::path(path_value.ToString())); | ||
| TraverseParent(realm, std::filesystem::path(path_value_str)); | ||
|
|
||
| if (package_json != nullptr) { | ||
| args.GetReturnValue().Set(package_json->Serialize(realm)); | ||
|
|
@@ -338,9 +347,22 @@ void BindingData::GetNearestParentPackageJSONType( | |
| CHECK(args[0]->IsString()); | ||
|
|
||
| Realm* realm = Realm::GetCurrent(args); | ||
| Utf8Value path(realm->isolate(), args[0]); | ||
| BufferValue path_value(realm->isolate(), args[0]); | ||
| // Check if the path has a trailing slash. If so, add it after | ||
| // ToNamespacedPath() as it will be deleted by ToNamespacedPath() | ||
| bool slashCheck = !path_value.ToString().empty() && | ||
| path_value.ToStringView().ends_with( | ||
| std::filesystem::path::preferred_separator); | ||
|
|
||
| ToNamespacedPath(realm->env(), &path_value); | ||
|
|
||
| std::string path_value_str = path_value.ToString(); | ||
| if (slashCheck) { | ||
| path_value_str.push_back(std::filesystem::path::preferred_separator); | ||
| } | ||
|
|
||
| auto package_json = | ||
| TraverseParent(realm, std::filesystem::path(path.ToString())); | ||
| TraverseParent(realm, std::filesystem::path(path_value_str)); | ||
|
|
||
| if (package_json == nullptr) { | ||
| return; | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| 'use strict'; | ||
|
|
||
| // Flags: --expose-internals | ||
|
|
||
| const common = require('../common'); | ||
| if (!common.isWindows) { | ||
| common.skip('this test is Windows-specific.'); | ||
| } | ||
|
|
||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
| const tmpdir = require('../common/tmpdir'); | ||
| const { describe, it } = require('node:test'); | ||
| const { legacyMainResolve } = require('node:internal/modules/esm/resolve'); | ||
| const { pathToFileURL } = require('node:url'); | ||
| const { spawnPromisified } = require('../common'); | ||
| const assert = require('assert'); | ||
| const { execPath } = require('node:process'); | ||
|
|
||
| describe('long path on Windows', () => { | ||
| it('check long path in ReadPackageJSON', () => { | ||
| // Module layout will be the following: | ||
| // package.json | ||
| // main | ||
| // main.js | ||
| const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1); | ||
| const packageDirName = tmpdir.resolve('x'.repeat(packageDirNameLen)); | ||
| const packageDirPath = path.resolve(packageDirName); | ||
| const packageJsonFilePath = path.join(packageDirPath, 'package.json'); | ||
| const mainDirName = 'main'; | ||
| const mainDirPath = path.resolve(packageDirPath, mainDirName); | ||
| const mainJsFile = 'main.js'; | ||
| const mainJsFilePath = path.resolve(mainDirPath, mainJsFile); | ||
|
|
||
| tmpdir.refresh(); | ||
|
|
||
| fs.mkdirSync(packageDirPath); | ||
| fs.writeFileSync( | ||
| packageJsonFilePath, | ||
| `{"main":"${mainDirName}/${mainJsFile}"}` | ||
| ); | ||
| fs.mkdirSync(mainDirPath); | ||
| fs.writeFileSync(mainJsFilePath, 'console.log("hello world");'); | ||
|
|
||
| require('internal/modules/run_main').executeUserEntryPoint(packageDirPath); | ||
|
|
||
| tmpdir.refresh(); | ||
| }); | ||
|
|
||
| it('check long path in LegacyMainResolve - 1', () => { | ||
| // Module layout will be the following: | ||
| // package.json | ||
| // index.js | ||
| const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1); | ||
| const packageDirPath = tmpdir.resolve('y'.repeat(packageDirNameLen)); | ||
| const packageJSPath = path.join(packageDirPath, 'package.json'); | ||
| const indexJSPath = path.join(packageDirPath, 'index.js'); | ||
| const packageConfig = { }; | ||
|
|
||
| tmpdir.refresh(); | ||
|
|
||
| fs.mkdirSync(packageDirPath); | ||
| fs.writeFileSync(packageJSPath, ''); | ||
| fs.writeFileSync(indexJSPath, ''); | ||
|
|
||
| const packageJsonUrl = pathToFileURL( | ||
| path.resolve(packageJSPath) | ||
| ); | ||
|
|
||
| console.log(legacyMainResolve(packageJsonUrl, packageConfig, '')); | ||
| }); | ||
|
|
||
| it('check long path in LegacyMainResolve - 2', () => { | ||
| // Module layout will be the following: | ||
| // package.json | ||
| // main.js | ||
| const packageDirNameLen = Math.max(260 - tmpdir.path.length, 1); | ||
| const packageDirPath = tmpdir.resolve('z'.repeat(packageDirNameLen)); | ||
| const packageJSPath = path.join(packageDirPath, 'package.json'); | ||
| const indexJSPath = path.join(packageDirPath, 'main.js'); | ||
| const packageConfig = { main: 'main.js' }; | ||
|
|
||
| tmpdir.refresh(); | ||
|
|
||
| fs.mkdirSync(packageDirPath); | ||
| fs.writeFileSync(packageJSPath, ''); | ||
| fs.writeFileSync(indexJSPath, ''); | ||
|
|
||
| const packageJsonUrl = pathToFileURL( | ||
| path.resolve(packageJSPath) | ||
| ); | ||
|
|
||
| console.log(legacyMainResolve(packageJsonUrl, packageConfig, '')); | ||
| }); | ||
|
|
||
| it('check long path in GetNearestParentPackageJSON', async () => { | ||
| // Module layout will be the following: | ||
| // node_modules | ||
| // test-module | ||
| // package.json (path is less than 260 chars long) | ||
| // cjs | ||
| // package.json (path is more than 260 chars long) | ||
| // index.js | ||
| const testModuleDirPath = 'node_modules/test-module'; | ||
| let cjsDirPath = path.join(testModuleDirPath, 'cjs'); | ||
| let modulePackageJSPath = path.join(testModuleDirPath, 'package.json'); | ||
| let cjsPackageJSPath = path.join(cjsDirPath, 'package.json'); | ||
| let cjsIndexJSPath = path.join(cjsDirPath, 'index.js'); | ||
|
|
||
| const tmpDirNameLen = Math.max( | ||
| 260 - tmpdir.path.length - cjsPackageJSPath.length, 1); | ||
| const tmpDirPath = tmpdir.resolve('k'.repeat(tmpDirNameLen)); | ||
|
|
||
| cjsDirPath = path.join(tmpDirPath, cjsDirPath); | ||
| modulePackageJSPath = path.join(tmpDirPath, modulePackageJSPath); | ||
| cjsPackageJSPath = path.join(tmpDirPath, cjsPackageJSPath); | ||
| cjsIndexJSPath = path.join(tmpDirPath, cjsIndexJSPath); | ||
|
|
||
| tmpdir.refresh(); | ||
|
|
||
| fs.mkdirSync(cjsDirPath, { recursive: true }); | ||
| fs.writeFileSync( | ||
| modulePackageJSPath, | ||
| `{ | ||
| "type": "module" | ||
| }` | ||
| ); | ||
| fs.writeFileSync( | ||
| cjsPackageJSPath, | ||
| `{ | ||
| "type": "commonjs" | ||
| }` | ||
| ); | ||
| fs.writeFileSync( | ||
| cjsIndexJSPath, | ||
| 'const fs = require("fs");' | ||
| ); | ||
| const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]); | ||
| assert.strictEqual(stderr.trim(), ''); | ||
| assert.strictEqual(code, 0); | ||
| assert.strictEqual(signal, null); | ||
| }); | ||
|
|
||
| it('check long path in GetNearestParentPackageJSONType', async () => { | ||
| // Module layout will be the following: | ||
| // node_modules | ||
| // test-module | ||
| // package.json (path is less than 260 chars long) | ||
| // cjs | ||
| // package.json (path is more than 260 chars long) | ||
| // index.js | ||
| const testModuleDirPath = 'node_modules/test-module'; | ||
| let cjsDirPath = path.join(testModuleDirPath, 'cjs'); | ||
| let modulePackageJSPath = path.join(testModuleDirPath, 'package.json'); | ||
| let cjsPackageJSPath = path.join(cjsDirPath, 'package.json'); | ||
| let cjsIndexJSPath = path.join(cjsDirPath, 'index.js'); | ||
|
|
||
| const tmpDirNameLen = Math.max(260 - tmpdir.path.length - cjsPackageJSPath.length, 1); | ||
| const tmpDirPath = tmpdir.resolve('l'.repeat(tmpDirNameLen)); | ||
|
|
||
| cjsDirPath = path.join(tmpDirPath, cjsDirPath); | ||
| modulePackageJSPath = path.join(tmpDirPath, modulePackageJSPath); | ||
| cjsPackageJSPath = path.join(tmpDirPath, cjsPackageJSPath); | ||
| cjsIndexJSPath = path.join(tmpDirPath, cjsIndexJSPath); | ||
|
|
||
| tmpdir.refresh(); | ||
|
|
||
| fs.mkdirSync(cjsDirPath, { recursive: true }); | ||
| fs.writeFileSync( | ||
| modulePackageJSPath, | ||
| `{ | ||
| "type": "module" | ||
| }` | ||
| ); | ||
| fs.writeFileSync( | ||
| cjsPackageJSPath, | ||
| `{ | ||
| "type": "commonjs" | ||
| }` | ||
| ); | ||
|
|
||
| fs.writeFileSync(cjsIndexJSPath, 'import fs from "node:fs/promises";'); | ||
| const { code, signal, stderr } = await spawnPromisified(execPath, [cjsIndexJSPath]); | ||
|
|
||
| assert.ok(stderr.includes('Warning: To load an ES module')); | ||
| assert.strictEqual(code, 1); | ||
| assert.strictEqual(signal, null); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.