Skip to content

Commit

Permalink
Guard against NullReference while creating HostContext (actions#2343)
Browse files Browse the repository at this point in the history
* Guard against NullReference while creating HostContext

* Throw on IOUtil loading object if content is empty or object is null

* removed unused dependency

* Changed exceptions to ArgumentNullException and ArgumentException

* fixed my mistake on LoadObject

* fixed tests

* trigger workflows

* trigger workflows

* encode files using utf8
  • Loading branch information
nikola-jokic authored Mar 30, 2023
1 parent 9f778b8 commit 1ceb1a6
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/Runner.Common/HostContext.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
Expand Down Expand Up @@ -220,7 +220,7 @@ public HostContext(string hostType, string logFile = null)
var runnerFile = GetConfigFile(WellKnownConfigFile.Runner);
if (File.Exists(runnerFile))
{
var runnerSettings = IOUtil.LoadObject<RunnerSettings>(runnerFile);
var runnerSettings = IOUtil.LoadObject<RunnerSettings>(runnerFile, true);
_userAgents.Add(new ProductInfoHeaderValue("RunnerId", runnerSettings.AgentId.ToString(CultureInfo.InvariantCulture)));
_userAgents.Add(new ProductInfoHeaderValue("GroupId", runnerSettings.PoolId.ToString(CultureInfo.InvariantCulture)));
}
Expand Down
13 changes: 11 additions & 2 deletions src/Runner.Sdk/Util/IOUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,19 @@ public static void SaveObject(object obj, string path)
File.WriteAllText(path, StringUtil.ConvertToJson(obj), Encoding.UTF8);
}

public static T LoadObject<T>(string path)
public static T LoadObject<T>(string path, bool required = false)
{
string json = File.ReadAllText(path, Encoding.UTF8);
return StringUtil.ConvertFromJson<T>(json);
if (required && string.IsNullOrEmpty(json))
{
throw new ArgumentNullException($"File {path} is empty");
}
T result = StringUtil.ConvertFromJson<T>(json);
if (required && result == null)
{
throw new ArgumentException("Converting json to object resulted in a null value");
}
return result;
}

public static string GetSha256Hash(string path)
Expand Down
31 changes: 30 additions & 1 deletion src/Test/L0/Util/IOUtilL0.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using GitHub.Runner.Common.Util;
using GitHub.Runner.Sdk;
using System;
using System.IO;
Expand Down Expand Up @@ -931,6 +930,36 @@ public void ValidateExecutePermission_ExceedsFailsafe()
}
}

[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Common")]
public void LoadObject_ThrowsOnRequiredLoadObject()
{
using (TestHostContext hc = new(this))
{
Tracing trace = hc.GetTrace();

// Arrange: Create a directory with a file.
string directory = Path.Combine(hc.GetDirectory(WellKnownDirectory.Bin), Path.GetRandomFileName());

string file = Path.Combine(directory, "empty file");
Directory.CreateDirectory(directory);

File.WriteAllText(path: file, contents: "");
Assert.Throws<ArgumentNullException>(() => IOUtil.LoadObject<RunnerSettings>(file, true));

file = Path.Combine(directory, "invalid type file");
File.WriteAllText(path: file, contents: " ");
Assert.Throws<ArgumentException>(() => IOUtil.LoadObject<RunnerSettings>(file, true));

// Cleanup.
if (Directory.Exists(directory))
{
Directory.Delete(directory, recursive: true);
}
}
}

private static async Task CreateDirectoryReparsePoint(IHostContext context, string link, string target)
{
#if OS_WINDOWS
Expand Down

0 comments on commit 1ceb1a6

Please sign in to comment.