Skip to content

Commit c205035

Browse files
committed
Restore node PATH override as a backup
Although NODE_OPTIONS should be sufficient more most cases, if users set their own and ignore the env var (e.g. in an npm script, which imo is reasonably common) then it won't be used. Keeping the path override has little or no downside, and provides a backup that lets us catch most of those cases too. The one place that won't work is Yarn usage with NODE_OPTIONS, which effectively ignores both env vars entirely. Don't do that.
1 parent f3ac752 commit c205035

File tree

4 files changed

+52
-9
lines changed

4 files changed

+52
-9
lines changed

overrides/js/prepend-node.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
/**
2-
* --require'd by node before loading any other modules.
2+
* --require'd by node before loading any other modules. The --require
3+
* option is injected both using NODE_OPTIONS and with a node wrapper in
4+
* PATH to handle various potential cases (node dedupes --require anyway).
5+
*
36
* This file sets up a global agent for the http & https modules,
47
* plus tweaks various other HTTP clients that need nudges, so they
58
* all correctly pick up the proxy from the environment.

overrides/path/node

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#!/usr/bin/env bash
2+
set -e
3+
4+
# Exclude ourselves from PATH, find the real node, then reset PATH
5+
PATH="${PATH//`dirname "$0"`:/}"
6+
real_node=`command -v node`
7+
PATH="`dirname "$0"`:$PATH"
8+
9+
PREPEND_PATH=`dirname "$0"`/../js/prepend-node.js
10+
11+
# Call node with the given arguments, prefixed with our extra logic
12+
if command -v winpty >/dev/null 2>&1; then
13+
winpty "$real_node" -r "$PREPEND_PATH" "$@"
14+
else
15+
"$real_node" -r "$PREPEND_PATH" "$@"
16+
fi

overrides/path/node.bat

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
@echo off
2+
SETLOCAL
3+
4+
REM Exclude ourselves from PATH within this script, to avoid recursing
5+
set ORIGINALPATH=%PATH%
6+
REM Get the current file's folder
7+
set THIS_PATH=%~dp0
8+
REM Strip the trailing slash from the folder
9+
set WRAPPER_FOLDER=%THIS_PATH:~0,-1%
10+
REM Remove that folder from PATH
11+
call set PATH=%%PATH:%WRAPPER_FOLDER%;=%%
12+
13+
REM Get the real node path, store it in %REAL_NODE%
14+
FOR /F "tokens=*" %%g IN ('where node') do (SET REAL_NODE=%%g)
15+
16+
REM Reset PATH, so its visible to node & subprocesses
17+
set PATH=%ORIGINALPATH%
18+
19+
REM Start Node for real, with an extra arg to inject our logic
20+
"%REAL_NODE%" -r "%WRAPPER_FOLDER%\..\js\prepend-node.js" %*

src/interceptors/terminal/terminal-env-overrides.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,14 @@ const PATH_VAR_SEPARATOR = process.platform === 'win32' ? ';' : ':';
99
export const OVERRIDES_DIR = path.join(APP_ROOT, 'overrides');
1010
const OVERRIDE_RUBYGEMS_PATH = path.join(OVERRIDES_DIR, 'gems');
1111
const OVERRIDE_PYTHONPATH = path.join(OVERRIDES_DIR, 'pythonpath');
12+
1213
const OVERRIDE_JS_SCRIPT = path.join(OVERRIDES_DIR, 'js', 'prepend-node.js');
14+
const NODE_OPTION = `--require ${
15+
// Avoid quoting except when necessary, because node 8 doesn't support quotes here
16+
OVERRIDE_JS_SCRIPT.includes(' ')
17+
? `"${OVERRIDE_JS_SCRIPT}"`
18+
: OVERRIDE_JS_SCRIPT
19+
}`;
1320

1421
export const OVERRIDE_BIN_PATH = path.join(OVERRIDES_DIR, 'path');
1522

@@ -72,14 +79,11 @@ export function getTerminalEnvVars(
7279
? `${OVERRIDE_PYTHONPATH}:${currentEnv.PYTHONPATH}`
7380
: OVERRIDE_PYTHONPATH,
7481

75-
// We use $NODE_OPTIONS to prepend our script into node. Notably this also
76-
// clears it, which is important, as _our_ NODE_OPTIONS aren't meant for
82+
// We use $NODE_OPTIONS to prepend our script into node. Notably this drops existing
83+
// env values, when using our env, because _our_ NODE_OPTIONS aren't meant for
7784
// subprocesses. Otherwise e.g. --max-http-header-size breaks old Node/Electron.
78-
'NODE_OPTIONS': `--require ${
79-
// Avoid quoting except when necessary, because node 8 doesn't support quotes here
80-
OVERRIDE_JS_SCRIPT.includes(' ')
81-
? `"${OVERRIDE_JS_SCRIPT}"`
82-
: OVERRIDE_JS_SCRIPT
83-
}`
85+
'NODE_OPTIONS': currentEnv === 'runtime-inherit'
86+
? `$NODE_OPTIONS ${NODE_OPTION}`
87+
: NODE_OPTION
8488
};
8589
}

0 commit comments

Comments
 (0)