From 5f1c6f4708dc4efeb4afdbda7936537504ef4d33 Mon Sep 17 00:00:00 2001 From: Ferenc Hammerl <31069338+fhammerl@users.noreply.github.com> Date: Tue, 20 Jun 2023 15:53:44 +0200 Subject: [PATCH] Add 'http://' to http(s)_proxy if there is no protocol (#2663) * Add 'http://' to http(s)_proxy if there is no protocol Before this commit, these URIs used to be ignored (nullproxy). The new behaviour aligns with go style http(s)_proxy * Update src/Runner.Sdk/RunnerWebProxy.cs Co-authored-by: Nikola Jokic * Assert that original protocol is preserved * Use startsWith for testing protocol * Only modify proxy if useful --------- Co-authored-by: Nikola Jokic --- src/Runner.Sdk/RunnerWebProxy.cs | 23 +++++++++++ src/Test/L0/RunnerWebProxyL0.cs | 66 +++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/Runner.Sdk/RunnerWebProxy.cs b/src/Runner.Sdk/RunnerWebProxy.cs index 680d40ef5..4c3c92a55 100644 --- a/src/Runner.Sdk/RunnerWebProxy.cs +++ b/src/Runner.Sdk/RunnerWebProxy.cs @@ -69,6 +69,10 @@ namespace GitHub.Runner.Sdk return; } + if (!string.IsNullOrEmpty(httpProxyAddress) && !Uri.TryCreate(httpProxyAddress, UriKind.Absolute, out var _)) + { + httpProxyAddress = PrependHttpIfMissing(httpProxyAddress); + } if (!string.IsNullOrEmpty(httpProxyAddress) && Uri.TryCreate(httpProxyAddress, UriKind.Absolute, out var proxyHttpUri)) { _httpProxyAddress = proxyHttpUri.OriginalString; @@ -99,6 +103,10 @@ namespace GitHub.Runner.Sdk } } + if (!string.IsNullOrEmpty(httpsProxyAddress) && !Uri.TryCreate(httpsProxyAddress, UriKind.Absolute, out var _)) + { + httpsProxyAddress = PrependHttpIfMissing(httpsProxyAddress); + } if (!string.IsNullOrEmpty(httpsProxyAddress) && Uri.TryCreate(httpsProxyAddress, UriKind.Absolute, out var proxyHttpsUri)) { _httpsProxyAddress = proxyHttpsUri.OriginalString; @@ -240,5 +248,20 @@ namespace GitHub.Runner.Sdk return false; } + + private string PrependHttpIfMissing(string proxyAddress) + { + // much like in golang, see https://github.com/golang/net/blob/f5464ddb689c015d1abf4df78a806a54af977e6c/http/httpproxy/proxy.go#LL156C31-L156C31 + if (!proxyAddress.StartsWith("http://", StringComparison.Ordinal) && !proxyAddress.StartsWith("https://", StringComparison.Ordinal)) + { + var prependedProxyAddress = "http://" + proxyAddress; + if (Uri.TryCreate(prependedProxyAddress, UriKind.Absolute, out var _)) + { + // if prepending http:// turns the proxyAddress into a valid Uri, then use that + return prependedProxyAddress; + } + } + return proxyAddress; + } } } diff --git a/src/Test/L0/RunnerWebProxyL0.cs b/src/Test/L0/RunnerWebProxyL0.cs index 68410aef6..61fe68d18 100644 --- a/src/Test/L0/RunnerWebProxyL0.cs +++ b/src/Test/L0/RunnerWebProxyL0.cs @@ -186,8 +186,8 @@ namespace GitHub.Runner.Common.Tests { try { - Environment.SetEnvironmentVariable("http_proxy", "127.0.0.1:7777"); - Environment.SetEnvironmentVariable("https_proxy", "127.0.0.1"); + Environment.SetEnvironmentVariable("http_proxy", "#fragment"); + Environment.SetEnvironmentVariable("https_proxy", "#fragment"); var proxy = new RunnerWebProxy(); Assert.Null(proxy.HttpProxyAddress); @@ -206,6 +206,68 @@ namespace GitHub.Runner.Common.Tests } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Common")] + public void WebProxyPrependsHTTPforHTTP_PROXY_IfNoProtocol() + { + try + { + Environment.SetEnvironmentVariable("http_proxy", "127.0.0.1:7777"); + var proxy = new RunnerWebProxy(); + + Assert.Equal("http://127.0.0.1:7777", proxy.HttpProxyAddress); + Assert.Null(proxy.HttpProxyUsername); + Assert.Null(proxy.HttpProxyPassword); + + Assert.Equal(0, proxy.NoProxyList.Count); + + Environment.SetEnvironmentVariable("http_proxy", "http://127.0.0.1:7777"); + proxy = new RunnerWebProxy(); + + Assert.Equal("http://127.0.0.1:7777", proxy.HttpProxyAddress); + Assert.Null(proxy.HttpProxyUsername); + Assert.Null(proxy.HttpProxyPassword); + + Assert.Equal(0, proxy.NoProxyList.Count); + } + finally + { + CleanProxyEnv(); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Common")] + public void WebProxyPrependsHTTPforHTTPS_PROXY_IfNoProtocol() + { + try + { + Environment.SetEnvironmentVariable("https_proxy", "127.0.0.1:7777"); + var proxy = new RunnerWebProxy(); + + Assert.Equal("http://127.0.0.1:7777", proxy.HttpsProxyAddress); + Assert.Null(proxy.HttpProxyUsername); + Assert.Null(proxy.HttpProxyPassword); + + Assert.Equal(0, proxy.NoProxyList.Count); + + Environment.SetEnvironmentVariable("https_proxy", "https://127.0.0.1:7777"); + proxy = new RunnerWebProxy(); + + Assert.Equal("https://127.0.0.1:7777", proxy.HttpsProxyAddress); // existing protocol 'https' is not removed + Assert.Null(proxy.HttpProxyUsername); + Assert.Null(proxy.HttpProxyPassword); + + Assert.Equal(0, proxy.NoProxyList.Count); + } + finally + { + CleanProxyEnv(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Common")]