Don't update the NODE_PATH when updating the environment on launch

Signed-off-by: Nathan Sobo <nathan@github.com>
This commit is contained in:
Max Brunsfeld 2016-08-03 15:50:21 -07:00 committed by Nathan Sobo
parent c381b93836
commit 4634091e9b
4 changed files with 109 additions and 255 deletions

View File

@ -1,159 +1,82 @@
'use babel'
/** @babel */
/* eslint-env jasmine */
import child_process from 'child_process'
import environmentHelpers from '../src/environment-helpers'
import os from 'os'
import updateProcessEnv from '../src/update-process-env'
import dedent from 'dedent'
describe('Environment handling', () => {
let originalEnv
let options
describe('updateProcessEnv(launchEnv)', function () {
let originalProcessEnv, originalProcessPlatform
beforeEach(() => {
originalEnv = process.env
delete process._originalEnv
options = {
platform: process.platform,
env: Object.assign({}, process.env)
}
beforeEach(function () {
originalProcessEnv = process.env
originalProcessPlatform = process.platform
process.env = {}
})
afterEach(() => {
process.env = originalEnv
delete process._originalEnv
afterEach(function () {
process.env = originalProcessEnv
process.platform = originalProcessPlatform
})
describe('on macOS, when PWD is not set', () => {
beforeEach(() => {
options.platform = 'darwin'
})
describe('when the launch environment appears to come from a shell', function () {
it('updates process.env to match the launch environment', function () {
process.env = {
WILL_BE_DELETED: 'hi',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
}
const initialProcessEnv = process.env
describe('needsPatching', () => {
it('returns true if PWD is unset', () => {
delete options.env.PWD
expect(environmentHelpers.needsPatching(options)).toBe(true)
options.env.PWD = undefined
expect(environmentHelpers.needsPatching(options)).toBe(true)
options.env.PWD = null
expect(environmentHelpers.needsPatching(options)).toBe(true)
options.env.PWD = false
expect(environmentHelpers.needsPatching(options)).toBe(true)
updateProcessEnv({PWD: '/the/dir', KEY1: 'value1', KEY2: 'value2'})
expect(process.env).toEqual({
PWD: '/the/dir',
KEY1: 'value1',
KEY2: 'value2',
NODE_ENV: 'the-node-env',
NODE_PATH: '/the/node/path',
})
it('returns false if PWD is set', () => {
options.env.PWD = 'xterm'
expect(environmentHelpers.needsPatching(options)).toBe(false)
})
})
describe('normalize', () => {
it('changes process.env if PWD is unset', () => {
if (process.platform === 'win32') {
return
}
delete options.env.PWD
environmentHelpers.normalize(options)
expect(process._originalEnv).toBeDefined()
expect(process._originalEnv).toBeTruthy()
expect(process.env).toBeDefined()
expect(process.env).toBeTruthy()
expect(process.env.PWD).toBeDefined()
expect(process.env.PWD).toBeTruthy()
expect(process.env.PATH).toBeDefined()
expect(process.env.PATH).toBeTruthy()
expect(process.env.ATOM_HOME).toBeDefined()
expect(process.env.ATOM_HOME).toBeTruthy()
})
// See #11302. On Windows, `process.env` is a magic object that offers
// case-insensitive environment variable matching, so we cannot replace it
// with another object.
expect(process.env).toBe(initialProcessEnv)
})
})
describe('on a platform other than macOS', () => {
beforeEach(() => {
options.platform = 'penguin'
})
describe('when the launch environment does not come from a shell', function () {
describe('on osx', function () {
it('updates process.env to match the environment in the user\'s login shell', function () {
process.platform = 'darwin'
process.env.SHELL = '/my/custom/bash'
describe('needsPatching', () => {
it('returns false if PWD is set or unset', () => {
delete options.env.PWD
expect(environmentHelpers.needsPatching(options)).toBe(false)
options.env.PWD = undefined
expect(environmentHelpers.needsPatching(options)).toBe(false)
options.env.PWD = null
expect(environmentHelpers.needsPatching(options)).toBe(false)
options.env.PWD = false
expect(environmentHelpers.needsPatching(options)).toBe(false)
options.env.PWD = '/'
expect(environmentHelpers.needsPatching(options)).toBe(false)
})
it('returns false for linux', () => {
options.platform = 'linux'
options.PWD = '/'
expect(environmentHelpers.needsPatching(options)).toBe(false)
})
it('returns false for windows', () => {
options.platform = 'win32'
options.PWD = 'c:\\'
expect(environmentHelpers.needsPatching(options)).toBe(false)
})
})
describe('normalize', () => {
it('does not change the environment', () => {
if (process.platform === 'win32') {
return
}
delete options.env.PWD
environmentHelpers.normalize(options)
expect(process._originalEnv).toBeUndefined()
expect(process.env).toBeDefined()
expect(process.env).toBeTruthy()
expect(process.env.PATH).toBeDefined()
expect(process.env.PATH).toBeTruthy()
expect(process.env.PWD).toBeUndefined()
expect(process.env.PATH).toBe(originalEnv.PATH)
expect(process.env.ATOM_HOME).toBeDefined()
expect(process.env.ATOM_HOME).toBeTruthy()
})
})
})
describe('getFromShell', () => {
describe('when things are configured properly', () => {
beforeEach(() => {
spyOn(child_process, 'spawnSync').andReturn({
stdout: 'FOO=BAR' + os.EOL + 'TERM=xterm-something' + os.EOL +
'PATH=/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path'
stdout: dedent`
FOO=BAR=BAZ=QUUX
TERM=xterm-something
PATH=/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path
`
})
updateProcessEnv(process.env)
expect(child_process.spawnSync.mostRecentCall.args[0]).toBe('/my/custom/bash')
expect(process.env).toEqual({
FOO: 'BAR=BAZ=QUUX',
TERM: 'xterm-something',
PATH: '/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path'
})
})
it('returns an object containing the information from the user\'s shell environment', () => {
let env = environmentHelpers.getFromShell()
expect(env.FOO).toEqual('BAR')
expect(env.TERM).toEqual('xterm-something')
expect(env.PATH).toEqual('/usr/bin:/bin:/usr/sbin:/sbin:/crazy/path')
})
})
describe('when an error occurs launching the shell', () => {
beforeEach(() => {
spyOn(child_process, 'spawnSync').andReturn({
error: new Error('testing when an error occurs')
})
})
describe('not on osx', function () {
it('does not update process.env', function () {
process.platform = 'win32'
spyOn(child_process, 'spawnSync')
process.env = {FOO: 'bar'}
it('returns undefined', () => {
expect(environmentHelpers.getFromShell()).toBeUndefined()
})
it('leaves the environment as-is when normalize() is called', () => {
options.platform = 'darwin'
delete options.env.PWD
expect(environmentHelpers.needsPatching(options)).toBe(true)
environmentHelpers.normalize(options)
expect(process.env).toBeDefined()
expect(process._originalEnv).toBeUndefined()
updateProcessEnv(process.env)
expect(child_process.spawnSync).not.toHaveBeenCalled()
expect(process.env).toEqual({FOO: 'bar'})
})
})
})

View File

@ -1,116 +0,0 @@
'use babel'
import {spawnSync} from 'child_process'
import os from 'os'
// Gets a dump of the user's configured shell environment.
//
// Returns the output of the `env` command or `undefined` if there was an error.
function getRawShellEnv () {
let shell = getUserShell()
// The `-ilc` set of options was tested to work with the macOS v10.11
// default-installed versions of bash, zsh, sh, and ksh. It *does not*
// work with csh or tcsh.
let results = spawnSync(shell, ['-ilc', 'env'], {encoding: 'utf8'})
if (results.error || !results.stdout || results.stdout.length <= 0) {
return
}
return results.stdout
}
function getUserShell () {
if (process.env.SHELL) {
return process.env.SHELL
}
return '/bin/bash'
}
// Gets the user's configured shell environment.
//
// Returns a copy of the user's shell enviroment.
function getFromShell () {
let shellEnvText = getRawShellEnv()
if (!shellEnvText) {
return
}
let env = {}
for (let line of shellEnvText.split(os.EOL)) {
if (line.includes('=')) {
let components = line.split('=')
if (components.length === 2) {
env[components[0]] = components[1]
} else {
let k = components.shift()
let v = components.join('=')
env[k] = v
}
}
}
return env
}
function needsPatching (options = { platform: process.platform, env: process.env }) {
if (options.platform === 'darwin' && !options.env.PWD) {
let shell = getUserShell()
if (shell.endsWith('csh') || shell.endsWith('tcsh') || shell.endsWith('fish')) {
return false
}
return true
}
return false
}
// Fix for #11302 because `process.env` on Windows is a magic object that offers case-insensitive
// environment variable matching. By always cloning to `process.env` we prevent breaking the
// underlying functionality.
function clone (to, from) {
for (var key in to) {
// Don't erase NODE_ENV. Fixes #12024
if (key !== 'NODE_ENV') {
delete to[key]
}
}
Object.assign(to, from)
}
function normalize (options = {}) {
if (options && options.env) {
clone(process.env, options.env)
}
if (!options.env) {
options.env = process.env
}
if (!options.platform) {
options.platform = process.platform
}
if (needsPatching(options)) {
// Patch the `process.env` on startup to fix the problem first documented
// in #4126. Retain the original in case someone needs it.
let shellEnv = getFromShell()
if (shellEnv && shellEnv.PATH) {
process._originalEnv = Object.assign({}, process.env)
clone(process.env, shellEnv)
}
}
}
function replace (env) {
if (!env || !env.PATH) {
return
}
clone(process.env, env)
}
export default { getFromShell, needsPatching, normalize, replace }

View File

@ -1,15 +1,13 @@
# Like sands through the hourglass, so are the days of our lives.
module.exports = ({blobStore}) ->
environmentHelpers = require('./environment-helpers')
updateProcessEnv = require('./update-process-env')
path = require 'path'
require './window'
{getWindowLoadSettings} = require './window-load-settings-helpers'
{ipcRenderer} = require 'electron'
{resourcePath, isSpec, devMode, env} = getWindowLoadSettings()
# Set baseline environment
environmentHelpers.normalize({env: env})
env = process.env
updateProcessEnv(env)
# Add application-specific exports to module search path.
exportsPath = path.join(resourcePath, 'exports')
@ -37,5 +35,5 @@ module.exports = ({blobStore}) ->
setTimeout (-> document.querySelector('atom-workspace').focus()), 0
window.addEventListener('focus', windowFocused)
ipcRenderer.on('environment', (event, env) ->
environmentHelpers.replace(env)
updateProcessEnv(env)
)

49
src/update-process-env.js Normal file
View File

@ -0,0 +1,49 @@
/** @babel */
import {spawnSync} from 'child_process'
const ENVIRONMENT_VARIABLES_TO_PRESERVE = new Set(['NODE_ENV', 'NODE_PATH'])
export default function updateProcessEnv (launchEnv) {
let envToAssign
if (launchEnv.PWD) {
envToAssign = launchEnv
} else {
if (process.platform === 'darwin') {
envToAssign = getEnvFromShell()
}
}
if (envToAssign) {
for (let key in process.env) {
if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) {
delete process.env[key]
}
}
for (let key in envToAssign) {
if (!ENVIRONMENT_VARIABLES_TO_PRESERVE.has(key)) {
process.env[key] = envToAssign[key]
}
}
}
}
function getEnvFromShell () {
let shell = process.env.SHELL
if (shell && (shell.endsWith('/bash') || shell.endsWith('/sh'))) {
let {stdout} = spawnSync(shell, ['-ilc', 'env'], {encoding: 'utf8'})
if (stdout) {
let result = {}
for (let line of stdout.split('\n')) {
if (line.includes('=')) {
let components = line.split('=')
let key = components.shift()
let value = components.join('=')
result[key] = value
}
}
return result
}
}
}