Skip to content

Commit 4ef0e91

Browse files
droyaddroyad
authored
Merge pull request #47 from borland/null-safety-2
Add more safety when getting directory paths from Environment Variables
2 parents 0a030cf + 58a50a2 commit 4ef0e91

15 files changed

+211
-29
lines changed

‎src/Assent.Tests/DirPathTests.cs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
using System;
2+
using System.IO;
3+
using FluentAssertions;
4+
using NUnit.Framework;
5+
6+
namespace Assent.Tests;
7+
8+
public static class DirPathTests
9+
{
10+
public class NoSubDirs
11+
{
12+
[Test]
13+
public void SuccessWithValueIfEnvVarSetAndDirExists()
14+
{
15+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
16+
getEnvironmentVariable: _ => "localAppDataFolder",
17+
directoryExists: _ => true);
18+
19+
value.Should().Be("localAppDataFolder");
20+
}
21+
22+
[Test]
23+
public void FailIfEnvVarNotSet()
24+
{
25+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
26+
getEnvironmentVariable: _ => null,
27+
directoryExists: _ => throw new InvalidOperationException("should not get here"));
28+
29+
value.Should().BeNull();
30+
}
31+
32+
[Test]
33+
public void FailIfEnvVarIsEmpty()
34+
{
35+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
36+
getEnvironmentVariable: _ => "",
37+
directoryExists: _ => throw new InvalidOperationException("should not get here"));
38+
39+
value.Should().BeNull();
40+
}
41+
42+
[Test]
43+
public void FailIfEnvVarSetAndDirDoesNotExist()
44+
{
45+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", [],
46+
getEnvironmentVariable: _ => "localAppDataFolder",
47+
directoryExists: _ => false);
48+
49+
value.Should().BeNull();
50+
}
51+
}
52+
53+
public class CombiningSubDirs
54+
{
55+
[Test]
56+
public void SuccessWithValueIfEnvVarSetAndDirExists()
57+
{
58+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
59+
getEnvironmentVariable: _ => "localAppDataFolder",
60+
directoryExists: _ => true);
61+
62+
value.Should().Be(Path.Combine("localAppDataFolder", "foo", "bar"));
63+
}
64+
65+
[Test]
66+
public void FailIfEnvVarNotSet()
67+
{
68+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
69+
getEnvironmentVariable: _ => null,
70+
directoryExists: _ => throw new InvalidOperationException("should not get here"));
71+
72+
value.Should().BeNull();
73+
}
74+
75+
[Test]
76+
public void FailIfEnvVarIsEmpty()
77+
{
78+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
79+
getEnvironmentVariable: _ => "",
80+
directoryExists: _ => throw new InvalidOperationException("should not get here"));
81+
82+
value.Should().BeNull();
83+
}
84+
85+
[Test]
86+
public void FailIfEnvVarSetAndDirDoesNotExist()
87+
{
88+
var value = DirPath.GetFromEnvironmentOrNull("LOCALAPPDATA", ["foo", "bar"],
89+
getEnvironmentVariable: _ => "localAppDataFolder",
90+
directoryExists: d =>
91+
{
92+
// it should check the existence of the combined subdir, NOT the base dir
93+
d.Should().Be(Path.Combine("localAppDataFolder", "foo", "bar"));
94+
return false;
95+
});
96+
97+
value.Should().BeNull();
98+
}
99+
}
100+
}

‎src/Assent.Tests/Namers/DefaultNamerFixture.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,22 @@ namespace Assent.Tests.Namers
66
{
77
public class DefaultNamerFixture
88
{
9-
[Test]
10-
public void NameIsCorrect()
9+
[Test, Platform("Win")]
10+
public void NameIsCorrectForWindowsPath()
1111
{
1212
new DefaultNamer()
1313
.GetName(new TestMetadata(this, "MyTest", @"c:\temp\source.cs"))
1414
.Should()
1515
.Be(@"c:\temp\DefaultNamerFixture.MyTest");
1616
}
17+
18+
[Test, Platform(Exclude = "Win")]
19+
public void NameIsCorrectForUnixPath()
20+
{
21+
new DefaultNamer()
22+
.GetName(new TestMetadata(this, "MyTest", "/tmp/source.cs"))
23+
.Should()
24+
.Be(@"/tmp/DefaultNamerFixture.MyTest");
25+
}
1726
}
1827
}

‎src/Assent.Tests/Namers/PostfixNamerFixture.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,22 @@ namespace Assent.Tests.Namers
66
{
77
public class PostfixNamerFixture
88
{
9-
[Test]
10-
public void NameIsCorrect()
9+
[Test, Platform("Win")]
10+
public void NameIsCorrectForWindowsPath()
1111
{
1212
new PostfixNamer("foo")
1313
.GetName(new TestMetadata(this, "MyTest", @"c:\temp\source.cs"))
1414
.Should()
1515
.Be(@"c:\temp\PostfixNamerFixture.MyTest.foo");
1616
}
17+
18+
[Test, Platform(Exclude = "Win")]
19+
public void NameIsCorrectForUnixPath()
20+
{
21+
new PostfixNamer("foo")
22+
.GetName(new TestMetadata(this, "MyTest", "/tmp/source.cs"))
23+
.Should()
24+
.Be("/tmp/PostfixNamerFixture.MyTest.foo");
25+
}
1726
}
1827
}

‎src/Assent.Tests/Namers/SubdirectoryNamerFixture.cs‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,40 @@ namespace Assent.Tests.Namers
66
{
77
public class SubdirectoryNamerFixture
88
{
9-
[Test]
10-
public void NameIsCorrect()
9+
[Test, Platform("Win")]
10+
public void NameIsCorrectForWindowsPath()
1111
{
1212
new SubdirectoryNamer("SubDir")
1313
.GetName(new TestMetadata(this, "MyTest", @"c:\temp\source.cs"))
1414
.Should()
1515
.Be(@"c:\temp\SubDir\SubdirectoryNamerFixture.MyTest");
1616
}
17+
18+
[Test, Platform(Exclude = "Win")]
19+
public void NameIsCorrectForUnixPath()
20+
{
21+
new SubdirectoryNamer("SubDir")
22+
.GetName(new TestMetadata(this, "MyTest", "/tmp/source.cs"))
23+
.Should()
24+
.Be("/tmp/SubDir/SubdirectoryNamerFixture.MyTest");
25+
}
1726

18-
[Test]
19-
public void NameIsCorrectWithPostfix()
27+
[Test, Platform("Win")]
28+
public void NameIsCorrectWithPostfixForWindowsPath()
2029
{
2130
new SubdirectoryNamer("SubDir", "foo")
2231
.GetName(new TestMetadata(this, "MyTest", @"c:\temp\source.cs"))
2332
.Should()
2433
.Be(@"c:\temp\SubDir\SubdirectoryNamerFixture.MyTest.foo");
2534
}
35+
36+
[Test, Platform(Exclude = "Win")]
37+
public void NameIsCorrectWithPostfixForUnixPath()
38+
{
39+
new SubdirectoryNamer("SubDir", "foo")
40+
.GetName(new TestMetadata(this, "MyTest", "/tmp/source.cs"))
41+
.Should()
42+
.Be("/tmp/SubDir/SubdirectoryNamerFixture.MyTest.foo");
43+
}
2644
}
2745
}

‎src/Assent/Assent.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,6 @@
1111
<PackageProjectUrl>https://github.com/droyad/Assent</PackageProjectUrl>
1212
<RepositoryUrl>https://github.com/droyad/Assent</RepositoryUrl>
1313
<PackageLicenseUrl>https://github.com/droyad/Assent/blob/master/LICENCE.md</PackageLicenseUrl>
14+
<LangVersion>latest</LangVersion>
1415
</PropertyGroup>
1516
</Project>

‎src/Assent/DirPath.cs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
using System;
2+
using System.IO;
3+
using System.Linq;
4+
5+
namespace Assent
6+
{
7+
internal static class DirPath
8+
{
9+
/// <summary>
10+
/// Reads the environment variable defined by `name`, appends the given subdirectories from `combineSubDirs` and checks that it exists using Directory.Exists.
11+
/// If no such directory exists, will return null. If it does exist, will return the environment variable value.
12+
/// </summary>
13+
internal static string GetFromEnvironmentOrNull(string name, params string[] combineSubDirs)
14+
=> GetFromEnvironmentOrNull(name, combineSubDirs, Environment.GetEnvironmentVariable, Directory.Exists);
15+
16+
// This method allows injected I/O functions for unit testing. Don't call it outside of tests
17+
internal static string GetFromEnvironmentOrNull(
18+
string name,
19+
string[] combineSubDirs,
20+
Func<string, string> getEnvironmentVariable,
21+
Func<string, bool> directoryExists)
22+
{
23+
var pathFromEnv = getEnvironmentVariable(name);
24+
if (string.IsNullOrWhiteSpace(pathFromEnv))
25+
return null;
26+
27+
var pathWithSubDirs = Path.Combine(new[] {pathFromEnv}.Concat(combineSubDirs).ToArray());
28+
29+
return directoryExists(pathWithSubDirs)
30+
? pathWithSubDirs
31+
: null;
32+
}
33+
}
34+
}

‎src/Assent/Reporters/DiffPrograms/BeyondCompareDiffProgram.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ static BeyondCompareDiffProgram()
1111
var paths = new List<string>();
1212
if (DiffReporter.IsWindows)
1313
{
14-
paths.AddRange(WindowsProgramFilePaths
14+
paths.AddRange(WindowsProgramFilePaths()
1515
.SelectMany(p =>
1616
new[]
1717
{

‎src/Assent/Reporters/DiffPrograms/DiffProgramBase.cs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,18 @@ namespace Assent.Reporters.DiffPrograms
88
{
99
public abstract class DiffProgramBase : IDiffProgram
1010
{
11-
protected static IReadOnlyList<string> WindowsProgramFilePaths => new[]
11+
protected static IReadOnlyList<string> WindowsProgramFilePaths()
12+
{
13+
var result = new[]
1214
{
13-
Environment.GetEnvironmentVariable("ProgramFiles"),
14-
Environment.GetEnvironmentVariable("ProgramFiles(x86)"),
15-
Environment.GetEnvironmentVariable("ProgramW6432"),
16-
Environment.GetEnvironmentVariable("LocalAppData") is string localAppData ? Path.Combine(localAppData, "Programs") : null
17-
}
18-
.Where(p => !string.IsNullOrWhiteSpace(p))
19-
.Distinct()
20-
.ToArray();
15+
DirPath.GetFromEnvironmentOrNull("ProgramFiles"),
16+
DirPath.GetFromEnvironmentOrNull("ProgramFiles(x86)"),
17+
DirPath.GetFromEnvironmentOrNull("ProgramW6432"),
18+
DirPath.GetFromEnvironmentOrNull("LocalAppData", "Programs")
19+
};
20+
21+
return result.Where(r => r != null).ToArray();
22+
}
2123

2224
public IReadOnlyList<string> SearchPaths { get; }
2325

‎src/Assent/Reporters/DiffPrograms/KDiff3DiffProgram.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ public class KDiff3DiffProgram : DiffProgramBase
88
{
99
static KDiff3DiffProgram()
1010
{
11-
DefaultSearchPaths = WindowsProgramFilePaths
11+
DefaultSearchPaths = WindowsProgramFilePaths()
1212
.Select(p => $@"{p}\KDiff3\KDiff3.exe")
1313
.ToArray();
1414
}

‎src/Assent/Reporters/DiffPrograms/MeldDiffProgram.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ static MeldDiffProgram()
1212
var paths = new List<string>();
1313
if (DiffReporter.IsWindows)
1414
{
15-
paths.AddRange(WindowsProgramFilePaths
15+
paths.AddRange(WindowsProgramFilePaths()
1616
.Select(p => $@"{p}\Meld\Meld.exe")
1717
.ToArray());
1818
}

0 commit comments

Comments
 (0)