module,win: fix long path resolve

PR-URL: https://github.com/nodejs/node/pull/53294
Fixes: https://github.com/nodejs/node/issues/50753
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

src/node_file.cc

@@ -3340,8 +3340,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
 
     for (int i = 0; i < legacy_main_extensions_with_main_end; i++) {
       file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
+      // TODO(anonrig): Remove this when ToNamespacedPath supports std::string
+      Local<Value> local_file_path =
+          Buffer::Copy(env->isolate(), file_path.c_str(), file_path.size())
+              .ToLocalChecked();
+      BufferValue buff_file_path(isolate, local_file_path);
+      ToNamespacedPath(env, &buff_file_path);
 
-      switch (FilePathIsFile(env, file_path)) {
+      switch (FilePathIsFile(env, buff_file_path.ToString())) {
         case BindingData::FilePathIsFileReturnType::kIsFile:
           return args.GetReturnValue().Set(i);
         case BindingData::FilePathIsFileReturnType::kIsNotFile:
@@ -3377,8 +3383,14 @@ void BindingData::LegacyMainResolve(const FunctionCallbackInfo<Value>& args) {
        i < legacy_main_extensions_package_fallback_end;
        i++) {
     file_path = *initial_file_path + std::string(legacy_main_extensions[i]);
+    // TODO(anonrig): Remove this when ToNamespacedPath supports std::string
+    Local<Value> local_file_path =
+        Buffer::Copy(env->isolate(), file_path.c_str(), file_path.size())
+            .ToLocalChecked();
+    BufferValue buff_file_path(isolate, local_file_path);
+    ToNamespacedPath(env, &buff_file_path);
 
-    switch (FilePathIsFile(env, file_path)) {
+    switch (FilePathIsFile(env, buff_file_path.ToString())) {
       case BindingData::FilePathIsFileReturnType::kIsFile:
         return args.GetReturnValue().Set(i);
       case BindingData::FilePathIsFileReturnType::kIsNotFile:

src/node_modules.cc

@@ -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
+  ToNamespacedPath(realm->env(), &path);
   auto package_json = GetPackageJSON(
-      realm, "\\\\?\\" + path.ToString(), is_esm ? &error_context : nullptr);
-#else
-  auto package_json =
-      GetPackageJSON(realm, path.ToString(), is_esm ? &error_context : nullptr);
-#endif
+      realm, path.ToStringView(), is_esm ? &error_context : nullptr);
+
   if (package_json == nullptr) {
     return;
   }
@@ -323,9 +319,21 @@ 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.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_value.ToString()));
+      TraverseParent(realm, std::filesystem::path(path_value_str));
 
   if (package_json != nullptr) {
     args.GetReturnValue().Set(package_json->Serialize(realm));
@@ -338,9 +346,21 @@ 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.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;

test/es-module/test-GH-50753.js

@@ -1,42 +0,0 @@
-'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');
-
-// https://github.com/nodejs/node/issues/50753
-// Make a path that is more than 260 chars long.
-// 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();

test/es-module/test-cjs-esm-warn.js

@@ -10,7 +10,7 @@ const { describe, it } = require('node:test');
 
 const requiringCjsAsEsm = path.resolve(fixtures.path('/es-modules/cjs-esm.js'));
 const requiringEsm = path.resolve(fixtures.path('/es-modules/cjs-esm-esm.js'));
-const pjson = path.resolve(
+const pjson = path.toNamespacedPath(
   fixtures.path('/es-modules/package-type-module/package.json')
 );
 

test/es-module/test-cjs-legacyMainResolve-permission.js

@@ -63,9 +63,11 @@ describe('legacyMainResolve', () => {
 
             assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
               code: 'ERR_ACCESS_DENIED',
-              resource: path.resolve(
-                ${JSON.stringify(fixtextureFolderEscaped)},
-                ${JSON.stringify(mainOrFolder)},
+              resource: path.toNamespacedPath(
+                path.resolve(
+                  ${JSON.stringify(fixtextureFolderEscaped)},
+                  ${JSON.stringify(mainOrFolder)},
+                )
               )
             });
           `,
@@ -120,10 +122,12 @@ describe('legacyMainResolve', () => {
 
             assert.throws(() => legacyMainResolve(packageJsonUrl, packageConfig, base), {
               code: 'ERR_ACCESS_DENIED',
-              resource: path.resolve(
-                ${JSON.stringify(fixtextureFolderEscaped)},
-                ${JSON.stringify(folder)},
-                ${JSON.stringify(expectedFile)},
+              resource: path.toNamespacedPath(
+                path.resolve(
+                  ${JSON.stringify(fixtextureFolderEscaped)},
+                  ${JSON.stringify(folder)},
+                  ${JSON.stringify(expectedFile)},
+                )
               )
             });
           `,

test/es-module/test-esm-long-path-win.js

@@ -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);
+  });
+});