Skip to content
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.

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

Guard against invalid path environment variables #1287

Closed
daveaglick opened this issue Oct 17, 2016 · 1 comment
Closed

Guard against invalid path environment variables #1287

daveaglick opened this issue Oct 17, 2016 · 1 comment
Milestone

Comments

@daveaglick
Copy link
Member

What You Are Seeing?

ArgumentException during tool resolution if user's PATH environment variable contains invalid paths according to System.IO.

What is Expected?

Skip the invalid paths.

What version of Cake are you using?

0.16.2

Are you running on a 32 or 64 bit system?

64

What environment are you running on? Windows? Linux? Mac?

Windows

Are you running on a CI Server? If so, which one?

No

How Did You Get This To Happen? (Steps to Reproduce)

Have an invalid path in my environment variable.

Output Log

Performing debug...
Attach debugger to process 6788 to continue
Debugger attached
Analyzing build script...
Analyzing e:/Code/CakeDocs/build.cake...
Processing build script...
Installing addins...
Installing NuGet package Octokit...
Error: System.ArgumentException: Illegal characters in path.
   at System.IO.Path.CheckInvalidPathChars(String path, Boolean checkAdditional)
   at System.IO.Path.IsPathRooted(String path)
   at Cake.Core.IO.Path..ctor(String path)
   at Cake.Core.IO.DirectoryPath..ctor(String path)
   at Cake.Core.Tooling.ToolResolutionStrategy.<>c.<GetPathDirectories>b__12_0(String p)
   at System.Linq.Enumerable.WhereSelectArrayIterator`2.MoveNext()
   at System.Collections.Generic.List`1.InsertRange(Int32 index, IEnumerable`1 collection)
   at Cake.Core.Tooling.ToolResolutionStrategy.GetPathDirectories()
   at Cake.Core.Tooling.ToolResolutionStrategy.LookInPath(String tool)
   at Cake.Core.Tooling.ToolResolutionStrategy.Resolve(IToolRepository repository, String tool)
   at Cake.Core.Tooling.ToolLocator.Resolve(String tool)
   at Cake.Core.IO.NuGet.NuGetToolResolver.ResolvePath()
   at Cake.NuGet.NuGetPackageInstaller.GetNuGetPath()
   at Cake.NuGet.NuGetPackageInstaller.Install(PackageReference package, PackageType type, DirectoryPath path)
   at Cake.Core.Scripting.ScriptProcessor.InstallAddins(ScriptAnalyzerResult analyzerResult, DirectoryPath installPath)
   at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments)
   at Cake.Commands.DebugCommand.Execute(CakeOptions options)
   at Cake.CakeApplication.Run(CakeOptions options)
   at Cake.Program.Main()

More Details

Tracked this to ToolResolutionStrategy.GetPathDirectories(). It makes an unguarded call to new DirectoryPath() for every path in the PATH environment variable. Since DirectoryPath defers some operations to System.IO if the path that was taken from the environment variable is not a valid path according to System.IO the exception will be thrown.

My proposal is to add a try-catch guard around the DirectoryPath construction and ignore any environment paths that throw. I'll submit a PR shortly for this.

@devlead
Copy link
Member

devlead commented Oct 17, 2016

Fixed by #1287

@devlead devlead closed this as completed Oct 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants