From b6ea5df08a42bd1d9bf71d3b00ec1c684463a5ec Mon Sep 17 00:00:00 2001
From: David Goldstein <dgoldstein@dropbox.com>
Date: Tue, 10 Apr 2018 07:40:56 +0000
Subject: [PATCH] module: add --preserve-symlinks-main

Add `--preserve-symlinks-main` option which behaves like
`--preserve-symlinks` but for `require.main`.

PR-URL: https://github.com/nodejs/node/pull/19911
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/cli.md                                | 23 ++++++++
 doc/node.1                                    |  5 +-
 lib/internal/modules/cjs/loader.js            | 17 +++++-
 lib/internal/modules/esm/default_resolve.js   |  3 +-
 src/node.cc                                   | 16 +++++-
 src/node_config.cc                            |  2 +
 src/node_internals.h                          |  5 ++
 .../test-esm-preserve-symlinks-main.js        | 57 +++++++++++++++++++
 8 files changed, 124 insertions(+), 4 deletions(-)
 create mode 100644 test/es-module/test-esm-preserve-symlinks-main.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 6d055d9d03a6dd..d76c49615c9bca 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -217,6 +217,29 @@ are linked from more than one location in the dependency tree (Node.js would
 see those as two separate modules and would attempt to load the module multiple
 times, causing an exception to be thrown).
 
+The `--preserve-symlinks` flag does not apply to the main module, which allows
+`node --preserve-symlinks node_module/.bin/<foo>` to work.  To apply the same
+behavior for the main module, also use `--preserve-symlinks-main`.
+
+### `--preserve-symlinks-main`
+<!-- YAML
+added: REPLACEME
+-->
+
+Instructs the module loader to preserve symbolic links when resolving and
+caching the main module (`require.main`).
+
+This flag exists so that the main module can be opted-in to the same behavior
+that `--preserve-symlinks` gives to all other imports; they are separate flags,
+however, for backward compatibility with older Node.js versions.
+
+Note that `--preserve-symlinks-main` does not imply `--preserve-symlinks`; it
+is expected that `--preserve-symlinks-main` will be used in addition to
+`--preserve-symlinks` when it is not desirable to follow symlinks before
+resolving relative paths.
+
+See `--preserve-symlinks` for more information.
+
 ### `--prof-process`
 <!-- YAML
 added: v5.2.0
diff --git a/doc/node.1 b/doc/node.1
index 238572c74c4796..f22e475d8465f2 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -143,7 +143,10 @@ Among other uses, this can be used to enable FIPS-compliant crypto if Node.js is
 Emit pending deprecation warnings.
 .
 .It Fl -preserve-symlinks
-Instructs the module loader to preserve symbolic links when resolving and caching modules.
+Instructs the module loader to preserve symbolic links when resolving and caching modules other than the main module.
+.
+.It F1 -preserve-symlinks-main
+Instructs the module loader to preserve symbolic links when resolving and caching the main module.
 .
 .It Fl -prof-process
 Process V8 profiler output generated using the V8 option
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index 8a4974225049ce..022f58d27e2868 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -42,6 +42,7 @@ const {
   stripShebang
 } = require('internal/modules/cjs/helpers');
 const preserveSymlinks = !!process.binding('config').preserveSymlinks;
+const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain;
 const experimentalModules = !!process.binding('config').experimentalModules;
 
 const {
@@ -239,7 +240,21 @@ Module._findPath = function(request, paths, isMain) {
     var rc = stat(basePath);
     if (!trailingSlash) {
       if (rc === 0) {  // File.
-        if (preserveSymlinks && !isMain) {
+        if (!isMain) {
+          if (preserveSymlinks) {
+            filename = path.resolve(basePath);
+          } else {
+            filename = toRealPath(basePath);
+          }
+        } else if (preserveSymlinksMain) {
+          // For the main module, we use the preserveSymlinksMain flag instead
+          // mainly for backward compatibility, as the preserveSymlinks flag
+          // historically has not applied to the main module.  Most likely this
+          // was intended to keep .bin/ binaries working, as following those
+          // symlinks is usually required for the imports in the corresponding
+          // files to resolve; that said, in some use cases following symlinks
+          // causes bigger problems which is why the preserveSymlinksMain option
+          // is needed.
           filename = path.resolve(basePath);
         } else {
           filename = toRealPath(basePath);
diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js
index 48d3ef73fd86cd..b573ce4e8d9e5a 100644
--- a/lib/internal/modules/esm/default_resolve.js
+++ b/lib/internal/modules/esm/default_resolve.js
@@ -7,6 +7,7 @@ const { NativeModule, internalBinding } = require('internal/bootstrap/loaders');
 const { extname } = require('path');
 const { realpathSync } = require('fs');
 const preserveSymlinks = !!process.binding('config').preserveSymlinks;
+const preserveSymlinksMain = !!process.binding('config').preserveSymlinksMain;
 const {
   ERR_MISSING_MODULE,
   ERR_MODULE_RESOLUTION_LEGACY,
@@ -71,7 +72,7 @@ function resolve(specifier, parentURL) {
 
   const isMain = parentURL === undefined;
 
-  if (!preserveSymlinks || isMain) {
+  if (isMain ? !preserveSymlinksMain : !preserveSymlinks) {
     const real = realpathSync(getPathFromURL(url), {
       [internalFS.realpathCacheKey]: realpathCache
     });
diff --git a/src/node.cc b/src/node.cc
index bcc3971bbe2fb7..09b1e34664c14c 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -236,6 +236,11 @@ bool trace_warnings = false;
 // that is used by lib/module.js
 bool config_preserve_symlinks = false;
 
+// Set in node.cc by ParseArgs when --preserve-symlinks-main is used.
+// Used in node_config.cc to set a constant on process.binding('config')
+// that is used by lib/module.js
+bool config_preserve_symlinks_main = false;
+
 // Set in node.cc by ParseArgs when --experimental-modules is used.
 // Used in node_config.cc to set a constant on process.binding('config')
 // that is used by lib/module.js
@@ -3519,6 +3524,8 @@ static void PrintHelp() {
          "  --pending-deprecation      emit pending deprecation warnings\n"
 #if defined(NODE_HAVE_I18N_SUPPORT)
          "  --preserve-symlinks        preserve symbolic links when resolving\n"
+         "  --preserve-symlinks-main   preserve symbolic links when resolving\n"
+         "                             the main module\n"
 #endif
          "  --prof-process             process v8 profiler output generated\n"
          "                             using --prof\n"
@@ -3569,7 +3576,6 @@ static void PrintHelp() {
          "  -r, --require              module to preload (option can be "
          "repeated)\n"
          "  -v, --version              print Node.js version\n"
-
          "\n"
          "Environment variables:\n"
          "NODE_DEBUG                   ','-separated list of core modules\n"
@@ -3832,6 +3838,8 @@ static void ParseArgs(int* argc,
       Revert(cve);
     } else if (strcmp(arg, "--preserve-symlinks") == 0) {
       config_preserve_symlinks = true;
+    } else if (strcmp(arg, "--preserve-symlinks-main") == 0) {
+      config_preserve_symlinks_main = true;
     } else if (strcmp(arg, "--experimental-modules") == 0) {
       config_experimental_modules = true;
       new_v8_argv[new_v8_argc] = "--harmony-dynamic-import";
@@ -4276,6 +4284,12 @@ void Init(int* argc,
         SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) && text[0] == '1';
   }
 
+  {
+    std::string text;
+    config_preserve_symlinks_main =
+        SafeGetenv("NODE_PRESERVE_SYMLINKS_MAIN", &text) && text[0] == '1';
+  }
+
   if (config_warning_file.empty())
     SafeGetenv("NODE_REDIRECT_WARNINGS", &config_warning_file);
 
diff --git a/src/node_config.cc b/src/node_config.cc
index e2b2662abd0e80..603d55491a259b 100644
--- a/src/node_config.cc
+++ b/src/node_config.cc
@@ -72,6 +72,8 @@ static void Initialize(Local<Object> target,
 
   if (config_preserve_symlinks)
     READONLY_BOOLEAN_PROPERTY("preserveSymlinks");
+  if (config_preserve_symlinks_main)
+    READONLY_BOOLEAN_PROPERTY("preserveSymlinksMain");
 
   if (config_experimental_modules) {
     READONLY_BOOLEAN_PROPERTY("experimentalModules");
diff --git a/src/node_internals.h b/src/node_internals.h
index e15df78ffdfee3..d52d38aa138ca1 100644
--- a/src/node_internals.h
+++ b/src/node_internals.h
@@ -173,6 +173,11 @@ extern std::string openssl_config;
 // that is used by lib/module.js
 extern bool config_preserve_symlinks;
 
+// Set in node.cc by ParseArgs when --preserve-symlinks-main is used.
+// Used in node_config.cc to set a constant on process.binding('config')
+// that is used by lib/module.js
+extern bool config_preserve_symlinks_main;
+
 // Set in node.cc by ParseArgs when --experimental-modules is used.
 // Used in node_config.cc to set a constant on process.binding('config')
 // that is used by lib/module.js
diff --git a/test/es-module/test-esm-preserve-symlinks-main.js b/test/es-module/test-esm-preserve-symlinks-main.js
new file mode 100644
index 00000000000000..fbca5dce743674
--- /dev/null
+++ b/test/es-module/test-esm-preserve-symlinks-main.js
@@ -0,0 +1,57 @@
+'use strict';
+
+const common = require('../common');
+const { spawn } = require('child_process');
+const assert = require('assert');
+const path = require('path');
+const fs = require('fs');
+
+const tmpdir = require('../common/tmpdir');
+tmpdir.refresh();
+const tmpDir = tmpdir.path;
+
+fs.mkdirSync(path.join(tmpDir, 'nested'));
+fs.mkdirSync(path.join(tmpDir, 'nested2'));
+
+const entry = path.join(tmpDir, 'nested', 'entry.js');
+const entry_link_absolute_path = path.join(tmpDir, 'link.js');
+const submodule = path.join(tmpDir, 'nested2', 'submodule.js');
+const submodule_link_absolute_path = path.join(tmpDir, 'submodule_link.js');
+
+fs.writeFileSync(entry, `
+const assert = require('assert');
+
+// this import only resolves with --preserve-symlinks-main set
+require('./submodule_link.js');
+`);
+fs.writeFileSync(submodule, '');
+
+try {
+  fs.symlinkSync(entry, entry_link_absolute_path);
+  fs.symlinkSync(submodule, submodule_link_absolute_path);
+} catch (err) {
+  if (err.code !== 'EPERM') throw err;
+  common.skip('insufficient privileges for symlinks');
+}
+
+function doTest(flags, done) {
+  // invoke the main file via a symlink.  In this case --preserve-symlinks-main
+  // dictates that it'll resolve relative imports in the main file relative to
+  // the symlink, and not relative to the symlink target; the file structure set
+  // up above requires this to not crash when loading ./submodule_link.js
+  spawn(process.execPath,
+        flags.concat([
+          '--preserve-symlinks',
+          '--preserve-symlinks-main', entry_link_absolute_path
+        ]),
+        { stdio: 'inherit' }).on('exit', (code) => {
+    assert.strictEqual(code, 0);
+    done();
+  });
+}
+
+// first test the commonjs module loader
+doTest([], () => {
+  // now test the new loader
+  doTest(['--experimental-modules'], () => {});
+});