Skip to content

PowerShell doesn't start if env var HOME is not defined #3437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lzybkr merged 1 commit into from
Apr 9, 2017
Merged

PowerShell doesn't start if env var HOME is not defined #3437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions src/System.Management.Automation/CoreCLR/CorePsExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,11 @@ private static string InternalGetFolderPath(SpecialFolder folder)
string folderPath = null;

#if UNIX
string envHome = System.Environment.GetEnvironmentVariable(Platform.CommonEnvVariableNames.Home);
if (null == envHome)
{
envHome = Platform.GetTemporaryDirectory();
}
switch (folder)
{
case SpecialFolder.ProgramFiles:
Expand All @@ -1060,11 +1065,22 @@ private static string InternalGetFolderPath(SpecialFolder folder)
if (!System.IO.Directory.Exists(folderPath)) { folderPath = null; }
break;
case SpecialFolder.Personal:
folderPath = System.Environment.GetEnvironmentVariable("HOME");
folderPath = envHome;
break;
case SpecialFolder.LocalApplicationData:
folderPath = System.IO.Path.Combine(System.Environment.GetEnvironmentVariable("HOME"), ".config");
if (!System.IO.Directory.Exists(folderPath)) { System.IO.Directory.CreateDirectory(folderPath); }
folderPath = System.IO.Path.Combine(envHome, ".config");
if (!System.IO.Directory.Exists(folderPath))
{
try
{
System.IO.Directory.CreateDirectory(folderPath);
}
catch (UnauthorizedAccessException)
{
// directory creation may fail if the account doesn't have filesystem permission such as some service accounts
folderPath = String.Empty;
}
}
break;
default:
throw new NotSupportedException();
Expand Down Expand Up @@ -1100,7 +1116,7 @@ private static string InternalGetFolderPath(SpecialFolder folder)
}
break;
case SpecialFolder.MyDocuments: // same as SpecialFolder.Personal
userProfile = System.Environment.GetEnvironmentVariable("USERPROFILE");
userProfile = System.Environment.GetEnvironmentVariable(Platform.CommonEnvVariableNames.Home);
if (userProfile != null)
{
folderPath = System.IO.Path.Combine(userProfile, "Documents");
Expand All @@ -1117,7 +1133,7 @@ private static string InternalGetFolderPath(SpecialFolder folder)
// It's guaranteed by NanoServer team that 'USERPROFILE' will be already set when SetupComplete.cmd runs.
// So we use the path '%USERPROFILE%\AppData\Local' as an alternative in this case, and also set the env
// variable %LOCALAPPDATA% to it, so that modules running in PS can depend on this env variable.
userProfile = System.Environment.GetEnvironmentVariable("USERPROFILE");
userProfile = System.Environment.GetEnvironmentVariable(Platform.CommonEnvVariableNames.Home);
if (userProfile != null)
{
string alternatePath = System.IO.Path.Combine(userProfile, @"AppData\Local");
Expand Down
52 changes: 49 additions & 3 deletions src/System.Management.Automation/CoreCLR/CorePsPlatform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace System.Management.Automation
/// </summary>
public static class Platform
{
private static string _tempDirectory = null;

/// <summary>
/// True if the current platform is Linux.
/// </summary>
Expand Down Expand Up @@ -204,6 +206,41 @@ internal static class CommonEnvVariableNames
#endif
}

/// <summary>
/// Remove the temporary directory created for the current process
/// </summary>
internal static void RemoveTemporaryDirectory()
{
if (null == _tempDirectory)
{
return;
}

try
{
Directory.Delete(_tempDirectory, true);
}
catch
{
// ignore if there is a failure
}
_tempDirectory = null;
}

/// <summary>
/// Get a temporary directory to use for the current process
/// </summary>
internal static string GetTemporaryDirectory()
{
if (null != _tempDirectory)
{
return _tempDirectory;
}

_tempDirectory = PsUtils.GetTemporaryDirectory();
return _tempDirectory;
}

#if UNIX
/// <summary>
/// X Desktop Group configuration type enum.
Expand Down Expand Up @@ -234,10 +271,15 @@ public static string SelectProductNameForDirectory(Platform.XDG_Type dirpath)
string xdgconfighome = System.Environment.GetEnvironmentVariable("XDG_CONFIG_HOME");
string xdgdatahome = System.Environment.GetEnvironmentVariable("XDG_DATA_HOME");
string xdgcachehome = System.Environment.GetEnvironmentVariable("XDG_CACHE_HOME");
string xdgConfigHomeDefault = Path.Combine(System.Environment.GetEnvironmentVariable("HOME"), ".config", "powershell");
string xdgDataHomeDefault = Path.Combine(System.Environment.GetEnvironmentVariable("HOME"), ".local", "share", "powershell");
string envHome = System.Environment.GetEnvironmentVariable(CommonEnvVariableNames.Home);
if (null == envHome)
{
envHome = GetTemporaryDirectory();
}
string xdgConfigHomeDefault = Path.Combine(envHome, ".config", "powershell");
string xdgDataHomeDefault = Path.Combine(envHome, ".local", "share", "powershell");
string xdgModuleDefault = Path.Combine(xdgDataHomeDefault, "Modules");
string xdgCacheDefault = Path.Combine(System.Environment.GetEnvironmentVariable("HOME"), ".cache", "powershell");
string xdgCacheDefault = Path.Combine(envHome, ".cache", "powershell");

switch (dirpath)
{
Expand Down Expand Up @@ -268,6 +310,7 @@ public static string SelectProductNameForDirectory(Platform.XDG_Type dirpath)
catch (UnauthorizedAccessException)
{
//service accounts won't have permission to create user folder
return GetTemporaryDirectory();
}
}
return xdgDataHomeDefault;
Expand All @@ -291,6 +334,7 @@ public static string SelectProductNameForDirectory(Platform.XDG_Type dirpath)
catch (UnauthorizedAccessException)
{
//service accounts won't have permission to create user folder
return GetTemporaryDirectory();
}
}
return xdgModuleDefault;
Expand All @@ -317,6 +361,7 @@ public static string SelectProductNameForDirectory(Platform.XDG_Type dirpath)
catch (UnauthorizedAccessException)
{
//service accounts won't have permission to create user folder
return GetTemporaryDirectory();
}
}

Expand All @@ -334,6 +379,7 @@ public static string SelectProductNameForDirectory(Platform.XDG_Type dirpath)
catch (UnauthorizedAccessException)
{
//service accounts won't have permission to create user folder
return GetTemporaryDirectory();
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/System.Management.Automation/engine/hostifaces/LocalConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,8 @@ protected override void Dispose(bool disposing)
RunspaceOpening = null;
}

Platform.RemoveTemporaryDirectory();

// Dispose the event manager
if (this.ExecutionContext != null && this.ExecutionContext.Events != null)
{
Expand Down
24 changes: 24 additions & 0 deletions src/System.Management.Automation/utils/PsUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,30 @@ internal static bool IsRunningOnProcessorArchitectureARM()
#endif
}

/// <summary>
/// Get a temporary directory to use, needs to be unique to avoid collision
/// </summary>
internal static string GetTemporaryDirectory()
{
string tempDir = String.Empty;
string tempPath = Path.GetTempPath();
do
{
tempDir = Path.Combine(tempPath,System.Guid.NewGuid().ToString());
Copy link
Copy Markdown
Contributor

@lzybkr lzybkr Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this scenario is common (no HOME), we'll eventually create a ton of these directories.

I think we should either:

  • Use a reproducible name
  • Remove the directory before the process exits

Copy link
Copy Markdown
Member Author

@SteveL-MSFT SteveL-MSFT Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know how common this will be, but cleaning up makes sense. Putting it in ConsoleHost.Dispose()?

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is a common scenario, then perhaps we should require to specify a temporary directory using one of the possible ways?

Copy link
Copy Markdown
Member Author

@SteveL-MSFT SteveL-MSFT Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iSazonov although not discoverable, for advanced uses you can define XDG_CONFIG_HOME, XDG_CACHE_HOME, and XDG_DATA_HOME env vars which PowerShell will use (on non-Windows). This was really only to support simple cases (like Puppet) to not put a barrier for adoption of PowerShell

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant that maybe we need to specify in the documentation that for proper PowerShell operation, users need to define (enumeration here) variables.

Copy link
Copy Markdown
Member Author

@SteveL-MSFT SteveL-MSFT Mar 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created doc bug MicrosoftDocs/PowerShell-Docs#1126

I don't think users should be required to create those env vars, we should just work in most cases

}
while (Directory.Exists(tempDir));
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove the cycle? If we believe that it cannot be infinite that means we believe that NewGuid is always globally unique and then the cycle is generally not needed.

Copy link
Copy Markdown
Member Author

@SteveL-MSFT SteveL-MSFT Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the risk of an infinite loop is slightly lower than the risk of NewGuid() not being unique although realistically, I expect both to be practically not a real issue but having the loop seems very slightly safer.

Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov Mar 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closed.


try
{
Directory.CreateDirectory(tempDir);
}
catch (UnauthorizedAccessException)
{
tempDir = String.Empty; // will become current working directory
}
return tempDir;
}

internal static string GetHostName()
{
// Note: non-windows CoreCLR does not support System.Net yet
Expand Down
6 changes: 6 additions & 0 deletions test/powershell/Host/ConsoleHost.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ foo
[int]$output | Should BeGreaterThan 0
}
}

Context "HOME environment variable" {
It "Should start if HOME is not defined" -skip:($IsWindows) {
bash -c "unset HOME;$powershell -c '1+1'" | Should BeExactly 2
}
}
}

Describe "Console host api tests" -Tag CI {
Expand Down