Skip to content

Commit

Permalink
Reland: Support multi-line environment variables
Browse files Browse the repository at this point in the history
A standard Fedora install comes with 2 multiple line environment variables.
Since `env` was previously split by '\n' this would break them, causing
errors in the output pane and in terminals launched through the file
explorer (see #3495).

The original commit didn't work on OSX since `env` does not support the --null
arg. This version can fail if a command line arg's 1+th line looks like an
environment variable. There is no easy way to prevent this since `process.env`
cannot be leveraged. Since the likelyhood of this happening is small, plus the
chance of it causing any significant issue is also small it's a reasonable
compromise for the time being.

Fixes #3928
Fixes #4672
  • Loading branch information
Tyriar committed Mar 25, 2016
1 parent abfb016 commit 3ed0dfa
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 14 deletions.
50 changes: 36 additions & 14 deletions src/vs/base/node/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,45 @@ export function getUserEnvironment(): TPromise<IEnv> {
return c({});
}

let result: IEnv = Object.create(null);
c(parseEnvOutput(buffer));
});
});
}

buffer.split('\n').forEach(line => {
let pos = line.indexOf('=');
if (pos > 0) {
let key = line.substring(0, pos);
let value = line.substring(pos + 1);
/**
* Parse output from `env`, attempting to retain any multiple-line variables.
*/
export function parseEnvOutput(output): IEnv {
let result: IEnv = Object.create(null);
let vars = output.split('\n');

if (!key || typeof result[key] === 'string') {
return;
}
// Rejoin lines to the preceeding line if it doesn't look like the line is a new variable
let current = 0;
for (let i = 1; i < vars.length; i++) {
if (vars[i].match(/^[\w_][\w\d_]*=/) === null) {
vars[current] += `\n${vars[i]}`;
} else {
vars[++current] = vars[i];
}
}

result[key] = value;
}
});
// Trim any remaining vars that had been moved
vars.length = current + 1;

c(result);
});
// Turn the array into a map
vars.forEach(line => {
let pos = line.indexOf('=');
if (pos > 0) {
let key = line.substring(0, pos);
let value = line.substring(pos + 1);

if (!key || typeof result[key] === 'string') {
return;
}

result[key] = value;
}
});

return result;
}
42 changes: 42 additions & 0 deletions src/vs/base/test/node/env.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

'use strict';

import * as assert from 'assert';
import env = require('vs/base/node/env');

suite('Env', () => {
test('Parses multi-line environment variables at end of env', function(done: () => void) {
let vars = env.parseEnvOutput("a=first\nb=multiple\nlines");

assert.equal(Object.keys(vars).length, 2);
assert.equal(vars['a'], "first");
assert.equal(vars['b'], "multiple\nlines");

done();
});

test('Parses multi-line environment variables at start of env', function(done: () => void) {
let vars = env.parseEnvOutput("a=multiple\nlines\nb=second");

assert.equal(Object.keys(vars).length, 2);
assert.equal(vars['a'], "multiple\nlines");
assert.equal(vars['b'], "second");

done();
});

test('Parses complex multi-line environment variables', function(done: () => void) {
let vars = env.parseEnvOutput("a=1\nb=\n23 =4\n_5c=56\n d=7\nE =8");

assert.equal(Object.keys(vars).length, 3);
assert.equal(vars['a'], "1");
assert.equal(vars['b'], "\n23 =4");
assert.equal(vars['_5c'], "56\n d=7\nE =8");

done();
});
});

0 comments on commit 3ed0dfa

Please sign in to comment.