Skip to content

Commit 2e80e09

Browse files
mbgmbg
committed
C#: Apply suggestions from code review for DependabotProxy 
1 parent ee7f0b0 commit 2e80e09

File tree

5 files changed

+45
-51
lines changed

5 files changed

+45
-51
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependabotProxy.cs

+28-41
Original file line numberDiff line numberDiff line change
@@ -9,84 +9,71 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
99
{
1010
public class DependabotProxy : IDisposable
1111
{
12-
private readonly string? host;
13-
private readonly string? port;
14-
private readonly FileInfo? certFile;
12+
private readonly string host;
13+
private readonly string port;
1514

1615
/// <summary>
1716
/// The full address of the Dependabot proxy, if available.
1817
/// </summary>
19-
internal readonly string? Address;
18+
internal string Address { get; }
2019
/// <summary>
2120
/// The path to the temporary file where the certificate is stored.
2221
/// </summary>
23-
internal readonly string? CertificatePath;
22+
internal string? CertificatePath { get; private set; }
2423
/// <summary>
2524
/// The certificate used for the Dependabot proxy.
2625
/// </summary>
27-
internal readonly X509Certificate2? Certificate;
26+
internal X509Certificate2? Certificate { get; private set; }
2827

29-
/// <summary>
30-
/// Gets a value indicating whether a Dependabot proxy is configured.
31-
/// </summary>
32-
internal bool IsConfigured => !string.IsNullOrEmpty(this.Address);
33-
34-
internal DependabotProxy(ILogger logger, TemporaryDirectory tempWorkingDirectory)
28+
internal static DependabotProxy? GetDependabotProxy(ILogger logger, TemporaryDirectory tempWorkingDirectory)
3529
{
3630
// Obtain and store the address of the Dependabot proxy, if available.
37-
this.host = Environment.GetEnvironmentVariable(EnvironmentVariableNames.ProxyHost);
38-
this.port = Environment.GetEnvironmentVariable(EnvironmentVariableNames.ProxyPort);
31+
var host = Environment.GetEnvironmentVariable(EnvironmentVariableNames.ProxyHost);
32+
var port = Environment.GetEnvironmentVariable(EnvironmentVariableNames.ProxyPort);
3933

4034
if (string.IsNullOrWhiteSpace(host) || string.IsNullOrWhiteSpace(port))
4135
{
4236
logger.LogInfo("No Dependabot proxy credentials are configured.");
43-
return;
37+
return null;
4438
}
4539

46-
this.Address = $"http://{this.host}:{this.port}";
47-
logger.LogInfo($"Dependabot proxy configured at {this.Address}");
40+
var result = new DependabotProxy(host, port);
41+
logger.LogInfo($"Dependabot proxy configured at {result.Address}");
4842

4943
// Obtain and store the proxy's certificate, if available.
5044
var cert = Environment.GetEnvironmentVariable(EnvironmentVariableNames.ProxyCertificate);
5145

52-
if (string.IsNullOrWhiteSpace(cert))
46+
if (!string.IsNullOrWhiteSpace(cert))
5347
{
5448
logger.LogInfo("No certificate configured for Dependabot proxy.");
55-
return;
56-
}
5749

58-
var certDirPath = new DirectoryInfo(Path.Join(tempWorkingDirectory.DirInfo.FullName, ".dependabot-proxy"));
59-
Directory.CreateDirectory(certDirPath.FullName);
50+
var certDirPath = new DirectoryInfo(Path.Join(tempWorkingDirectory.DirInfo.FullName, ".dependabot-proxy"));
51+
Directory.CreateDirectory(certDirPath.FullName);
52+
53+
result.CertificatePath = Path.Join(certDirPath.FullName, "proxy.crt");
54+
var certFile = new FileInfo(result.CertificatePath);
6055

61-
this.CertificatePath = Path.Join(certDirPath.FullName, "proxy.crt");
62-
this.certFile = new FileInfo(this.CertificatePath);
56+
using var writer = certFile.CreateText();
57+
writer.Write(cert);
6358

64-
using var writer = this.certFile.CreateText();
65-
writer.Write(cert);
59+
logger.LogInfo($"Stored Dependabot proxy certificate at {result.CertificatePath}");
6660

67-
logger.LogInfo($"Stored Dependabot proxy certificate at {this.CertificatePath}");
61+
result.Certificate = new X509Certificate2(result.CertificatePath);
62+
}
6863

69-
this.Certificate = new X509Certificate2(this.CertificatePath);
64+
return result;
7065
}
7166

72-
internal void ApplyProxy(ILogger logger, ProcessStartInfo startInfo)
67+
private DependabotProxy(string host, string port)
7368
{
74-
// If the proxy isn't configured, we have nothing to do.
75-
if (!this.IsConfigured) return;
76-
77-
logger.LogInfo($"Setting up Dependabot proxy at {this.Address}");
78-
79-
startInfo.EnvironmentVariables.Add("HTTP_PROXY", this.Address);
80-
startInfo.EnvironmentVariables.Add("HTTPS_PROXY", this.Address);
81-
startInfo.EnvironmentVariables.Add("SSL_CERT_FILE", this.certFile?.FullName);
69+
this.host = host;
70+
this.port = port;
71+
this.Address = $"http://{this.host}:{this.port}";
8272
}
8373

8474
public void Dispose()
8575
{
86-
if (this.Certificate != null)
87-
{
88-
this.Certificate.Dispose();
89-
}
76+
this.Certificate?.Dispose();
9077
}
9178
}
9279
}

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public sealed partial class DependencyManager : IDisposable, ICompilationInfoCon
2727
private readonly ILogger logger;
2828
private readonly IDiagnosticsWriter diagnosticsWriter;
2929
private readonly NugetPackageRestorer nugetPackageRestorer;
30-
private readonly DependabotProxy dependabotProxy;
30+
private readonly DependabotProxy? dependabotProxy;
3131
private readonly IDotNet dotnet;
3232
private readonly FileContent fileContent;
3333
private readonly FileProvider fileProvider;
@@ -107,7 +107,7 @@ void exitCallback(int ret, string msg, bool silent)
107107
return BuildScript.Success;
108108
}).Run(SystemBuildActions.Instance, startCallback, exitCallback);
109109

110-
dependabotProxy = new DependabotProxy(logger, tempWorkingDirectory);
110+
dependabotProxy = DependabotProxy.GetDependabotProxy(logger, tempWorkingDirectory);
111111

112112
try
113113
{

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@ private DotNet(IDotNetCliInvoker dotnetCliInvoker, ILogger logger, TemporaryDire
2727
Info();
2828
}
2929

30-
private DotNet(ILogger logger, string? dotNetPath, TemporaryDirectory tempWorkingDirectory, DependabotProxy dependabotProxy) : this(new DotNetCliInvoker(logger, Path.Combine(dotNetPath ?? string.Empty, "dotnet"), dependabotProxy), logger, tempWorkingDirectory) { }
30+
private DotNet(ILogger logger, string? dotNetPath, TemporaryDirectory tempWorkingDirectory, DependabotProxy? dependabotProxy) : this(new DotNetCliInvoker(logger, Path.Combine(dotNetPath ?? string.Empty, "dotnet"), dependabotProxy), logger, tempWorkingDirectory) { }
3131

3232
internal static IDotNet Make(IDotNetCliInvoker dotnetCliInvoker, ILogger logger) => new DotNet(dotnetCliInvoker, logger);
3333

34-
public static IDotNet Make(ILogger logger, string? dotNetPath, TemporaryDirectory tempWorkingDirectory, DependabotProxy dependabotProxy) => new DotNet(logger, dotNetPath, tempWorkingDirectory, dependabotProxy);
34+
public static IDotNet Make(ILogger logger, string? dotNetPath, TemporaryDirectory tempWorkingDirectory, DependabotProxy? dependabotProxy) => new DotNet(logger, dotNetPath, tempWorkingDirectory, dependabotProxy);
3535

3636
private void Info()
3737
{

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNetCliInvoker.cs

+10-3
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching
1212
internal sealed class DotNetCliInvoker : IDotNetCliInvoker
1313
{
1414
private readonly ILogger logger;
15-
private readonly DependabotProxy proxy;
15+
private readonly DependabotProxy? proxy;
1616

1717
public string Exec { get; }
1818

19-
public DotNetCliInvoker(ILogger logger, string exec, DependabotProxy dependabotProxy)
19+
public DotNetCliInvoker(ILogger logger, string exec, DependabotProxy? dependabotProxy)
2020
{
2121
this.logger = logger;
2222
this.proxy = dependabotProxy;
@@ -42,7 +42,14 @@ private ProcessStartInfo MakeDotnetStartInfo(string args, string? workingDirecto
4242
startInfo.EnvironmentVariables["DOTNET_SKIP_FIRST_TIME_EXPERIENCE"] = "true";
4343

4444
// Configure the proxy settings, if applicable.
45-
this.proxy.ApplyProxy(this.logger, startInfo);
45+
if (this.proxy != null)
46+
{
47+
logger.LogInfo($"Setting up Dependabot proxy at {this.proxy.Address}");
48+
49+
startInfo.EnvironmentVariables.Add("HTTP_PROXY", this.proxy.Address);
50+
startInfo.EnvironmentVariables.Add("HTTPS_PROXY", this.proxy.Address);
51+
startInfo.EnvironmentVariables.Add("SSL_CERT_FILE", this.proxy.CertificatePath);
52+
}
4653

4754
return startInfo;
4855
}

‎csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal sealed partial class NugetPackageRestorer : IDisposable
2222
private readonly FileProvider fileProvider;
2323
private readonly FileContent fileContent;
2424
private readonly IDotNet dotnet;
25-
private readonly DependabotProxy dependabotProxy;
25+
private readonly DependabotProxy? dependabotProxy;
2626
private readonly IDiagnosticsWriter diagnosticsWriter;
2727
private readonly TemporaryDirectory legacyPackageDirectory;
2828
private readonly TemporaryDirectory missingPackageDirectory;
@@ -35,7 +35,7 @@ public NugetPackageRestorer(
3535
FileProvider fileProvider,
3636
FileContent fileContent,
3737
IDotNet dotnet,
38-
DependabotProxy dependabotProxy,
38+
DependabotProxy? dependabotProxy,
3939
IDiagnosticsWriter diagnosticsWriter,
4040
ILogger logger,
4141
ICompilationInfoContainer compilationInfoContainer)
@@ -596,7 +596,7 @@ private bool IsFeedReachable(string feed, int timeoutMilliSeconds, int tryCount,
596596

597597
// Configure the HttpClient to be aware of the Dependabot Proxy, if used.
598598
HttpClientHandler httpClientHandler = new();
599-
if (this.dependabotProxy.IsConfigured)
599+
if (this.dependabotProxy != null)
600600
{
601601
httpClientHandler.Proxy = new WebProxy(this.dependabotProxy.Address);
602602

0 commit comments

Comments
 (0)