From 9f95616d10fd3575b2fd20f745e00a8d44d5d41a Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Tue, 4 Nov 2025 15:23:56 +0100 Subject: [PATCH] Improve HttpEnvironmentProxy uri parsing --- .../HttpEnvironmentProxy.cs | 35 ++++++++----------- .../UnitTests/HttpEnvironmentProxyTest.cs | 8 +++++ 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpEnvironmentProxy.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpEnvironmentProxy.cs index 629b212f458107..dab4ad49f7b0dd 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpEnvironmentProxy.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpEnvironmentProxy.cs @@ -1,11 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Net; -using System.Net.Http; - namespace System.Net.Http { internal sealed class HttpEnvironmentProxyCredentials : ICredentials @@ -115,10 +110,11 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList } /// - /// This function will evaluate given string and it will try to convert - /// it to Uri object. The string could contain URI fragment, IP address and port - /// tuple or just IP address or name. It will return null if parsing fails. + /// Attempt to parse a partial Uri string into a Uri object. + /// The string may contain the scheme, user info, host, and port. The host is the only required part. + /// Example expected inputs: contoso.com, contoso.com:8080, http://contoso.com/, user@contoso.com. /// + /// if parsing fails. private static Uri? GetUriFromString(string? value) { if (string.IsNullOrEmpty(value)) @@ -187,6 +183,14 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList } } + // We expect inputs to not contain the path/query/fragment, but we do handle some simple cases like a trailing slash. + // An arbitrary path can break this parsing logic, but we expect environment variables to be trusted and well formed. + int delimiterIndex = value.IndexOfAny('/', '?', '#'); + if (delimiterIndex >= 0) + { + value = value.Substring(0, delimiterIndex); + } + int ipV6AddressEnd = value.IndexOf(']'); separatorIndex = value.LastIndexOf(':'); // No ':' or it is part of IPv6 address. @@ -197,18 +201,8 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList else { host = value.Substring(0, separatorIndex); - int endIndex = separatorIndex + 1; - // Strip any trailing characters after port number. - while (endIndex < value.Length) - { - if (!char.IsDigit(value[endIndex])) - { - break; - } - endIndex += 1; - } - if (!ushort.TryParse(value.AsSpan(separatorIndex + 1, endIndex - separatorIndex - 1), out port)) + if (!ushort.TryParse(value.AsSpan(separatorIndex + 1), out port)) { return null; } @@ -261,8 +255,7 @@ private bool IsMatchInBypassList(Uri input) { // This should match either domain it self or any subdomain or host // .foo.com will match foo.com it self or *.foo.com - if ((s.Length - 1) == input.Host.Length && - string.Compare(s, 1, input.Host, 0, input.Host.Length, StringComparison.OrdinalIgnoreCase) == 0) + if (s.AsSpan(1).Equals(input.Host, StringComparison.OrdinalIgnoreCase)) { return true; } diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpEnvironmentProxyTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpEnvironmentProxyTest.cs index 9e91d2743e8b92..fcadc1de12fc5f 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HttpEnvironmentProxyTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpEnvironmentProxyTest.cs @@ -137,14 +137,22 @@ await RemoteExecutor.Invoke(() => [InlineData("[::1]", "[::1]", "80", null, null)] [InlineData("domain\\foo:PLACEHOLDER@1.1.1.1", "1.1.1.1", "80", "foo", "PLACEHOLDER")] [InlineData("domain%5Cfoo:PLACEHOLDER@1.1.1.1", "1.1.1.1", "80", "foo", "PLACEHOLDER")] + [InlineData("placeholder@1.2.3.4/foo", "1.2.3.4", "80", "placeholder", "")] + [InlineData("placeholder@1.2.3?.4", "1.2.0.3", "80", "placeholder", "")] + [InlineData("placeholder:@1.2.3.4/foo:", "1.2.3.4", "80", "placeholder", "")] [InlineData("HTTP://ABC.COM/", "abc.com", "80", null, null)] [InlineData("http://10.30.62.64:7890/", "10.30.62.64", "7890", null, null)] + [InlineData("http://1.2.3.4/foo", "1.2.3.4", "80", null, null)] + [InlineData("http://1.2.3.4/foo:", "1.2.3.4", "80", null, null)] + [InlineData("http://1.2.3.4?foo", "1.2.3.4", "80", null, null)] [InlineData("http://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)] [InlineData("socks4://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)] [InlineData("socks4a://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)] [InlineData("socks5://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)] [InlineData("https://1.1.1.5:3005", "1.1.1.5", "3005", null, null)] [InlineData("https://1.1.1.5", "1.1.1.5", "443", null, null)] + // Everything before the last '@' is considered as user info (unlike regular Uri parsing). + [InlineData("https://host1:123@foo/@host2/path", "host2", "443", "host1", "123@foo/")] public async Task HttpProxy_Uri_Parsing(string _input, string _host, string _port, string? _user, string? _password) { await RemoteExecutor.Invoke((input, host, port, user, password) =>