Remove Uri scheme validation from HttpRequestMessage (#55035)

* Remove Uri scheme validation from HttpRequestMessage

* PR feedback

Move HttpUtilities to SocketsHttpHandler

* Add request scheme check to WinHttpHandler

* Skip .NET 6 specific WinHttpHandler test on Framework

* Update InteropServices.JavaScript HttpRequestMessage test

* Guard HttpMessageInvoker from calling HttpTelemetry with invalid Uris

* PR feedback

src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs

@@ -1950,5 +1950,23 @@ public async Task GetAsync_InvalidUrl_ExpectedExceptionThrown()
                 await Assert.ThrowsAsync<HttpRequestException>(() => client.GetStringAsync(invalidUri));
             }
         }
+
+        // HttpRequestMessage ctor guards against such Uris before .NET 6. We allow passing relative/unknown Uris to BrowserHttpHandler.
+        public static bool InvalidRequestUriTest_IsSupported => PlatformDetection.IsNotNetFramework && PlatformDetection.IsNotBrowser;
+
+        [ConditionalFact(nameof(InvalidRequestUriTest_IsSupported))]
+        public async Task SendAsync_InvalidRequestUri_Throws()
+        {
+            using var invoker = new HttpMessageInvoker(CreateHttpClientHandler());
+
+            var request = new HttpRequestMessage(HttpMethod.Get, (Uri)null);
+            await Assert.ThrowsAsync<InvalidOperationException>(() => invoker.SendAsync(request, CancellationToken.None));
+
+            request = new HttpRequestMessage(HttpMethod.Get, new Uri("/relative", UriKind.Relative));
+            await Assert.ThrowsAsync<InvalidOperationException>(() => invoker.SendAsync(request, CancellationToken.None));
+
+            request = new HttpRequestMessage(HttpMethod.Get, new Uri("foo://foo.bar"));
+            await Assert.ThrowsAsync<NotSupportedException>(() => invoker.SendAsync(request, CancellationToken.None));
+        }
     }
 }

src/libraries/System.Net.Http.WinHttpHandler/src/Resources/Strings.resx

@@ -126,4 +126,10 @@
   <data name="PlatformNotSupported_WinHttpHandler" xml:space="preserve">
     <value>WinHttpHandler is only supported on .NET Framework and .NET runtimes on Windows. It is not supported for Windows Store Applications (UWP) or Unix platforms.</value>
   </data>
+  <data name="net_http_unsupported_requesturi_scheme" xml:space="preserve">
+    <value>The '{0}' scheme is not supported.</value>
+  </data>
+  <data name="net_http_client_invalid_requesturi" xml:space="preserve">
+    <value>An invalid request URI was provided. Either the request URI must be an absolute URI or BaseAddress must be set.</value>
+  </data>
 </root>

src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs

@@ -578,6 +578,17 @@ protected override Task<HttpResponseMessage> SendAsync(
                 throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest);
             }
 
+            Uri? requestUri = request.RequestUri;
+            if (requestUri is null || !requestUri.IsAbsoluteUri)
+            {
+                throw new InvalidOperationException(SR.net_http_client_invalid_requesturi);
+            }
+
+            if (requestUri.Scheme != Uri.UriSchemeHttp && requestUri.Scheme != Uri.UriSchemeHttps)
+            {
+                throw new NotSupportedException(SR.Format(SR.net_http_unsupported_requesturi_scheme, requestUri.Scheme));
+            }
+
             // Check for invalid combinations of properties.
             if (_proxy != null && _windowsProxyUsePolicy != WindowsProxyUsePolicy.UseCustomProxy)
             {

src/libraries/System.Net.Http/src/Resources/Strings.resx

@@ -196,13 +196,10 @@
     <value>The base address must be an absolute URI.</value>
   </data>
   <data name="net_http_client_invalid_requesturi" xml:space="preserve">
-    <value>An invalid request URI was provided. The request URI must either be an absolute URI or BaseAddress must be set.</value>
+    <value>An invalid request URI was provided. Either the request URI must be an absolute URI or BaseAddress must be set.</value>
   </data>
-  <data name="net_http_client_http_baseaddress_required" xml:space="preserve">
-    <value>Only 'http' and 'https' schemes are allowed.</value>
-  </data>
-  <data name="net_http_client_http_browser_baseaddress_required" xml:space="preserve">
-    <value>Only 'http', 'https', and 'blob' schemes are allowed.</value>
+  <data name="net_http_unsupported_requesturi_scheme" xml:space="preserve">
+    <value>The '{0}' scheme is not supported.</value>
   </data>
   <data name="net_http_parser_invalid_base64_string" xml:space="preserve">
     <value>Value '{0}' is not a valid Base64 string. Error: {1}</value>

src/libraries/System.Net.Http/src/System.Net.Http.csproj

@@ -53,7 +53,6 @@
     <Compile Include="System\Net\Http\HttpResponseMessage.cs" />
     <Compile Include="System\Net\Http\HttpRuleParser.cs" />
     <Compile Include="System\Net\Http\HttpTelemetry.cs" />
-    <Compile Include="System\Net\Http\HttpUtilities.cs" />
     <Compile Include="System\Net\Http\HttpVersionPolicy.cs" />
     <Compile Include="System\Net\Http\MessageProcessingHandler.cs" />
     <Compile Include="System\Net\Http\MultipartContent.cs" />
@@ -180,6 +179,7 @@
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpContentStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpContentWriteStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpKeepAlivePingPolicy.cs" />
+    <Compile Include="System\Net\Http\SocketsHttpHandler\HttpUtilities.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\IHttpTrace.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\IMultiWebProxy.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\MultiProxy.cs" />
@@ -188,7 +188,6 @@
     <Compile Include="System\Net\Http\SocketsHttpHandler\SocketsHttpConnectionContext.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\SocketsHttpHandler.cs" />
     <Compile Include="System\Net\Http\HttpTelemetry.AnyOS.cs" />
-    <Compile Include="System\Net\Http\HttpUtilities.AnyOS.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\SystemProxyInfo.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\SocksHelper.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\SocksException.cs" />
@@ -623,7 +622,6 @@
     <Compile Include="System\Net\Http\BrowserHttpHandler\SocketsHttpHandler.cs" />
     <Compile Include="System\Net\Http\BrowserHttpHandler\BrowserHttpHandler.cs" />
     <Compile Include="System\Net\Http\BrowserHttpHandler\HttpTelemetry.Browser.cs" />
-    <Compile Include="System\Net\Http\BrowserHttpHandler\HttpUtilities.Browser.cs" />
     <Compile Include="$(CommonPath)System\Net\Http\HttpHandlerDefaults.cs"
              Link="Common\System\Net\Http\HttpHandlerDefaults.cs" />
   </ItemGroup>

src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs

@@ -26,8 +26,8 @@ public partial class HttpClient : HttpMessageInvoker
 
         private CancellationTokenSource _pendingRequestsCts;
         private HttpRequestHeaders? _defaultRequestHeaders;
-        private Version _defaultRequestVersion = HttpUtilities.DefaultRequestVersion;
-        private HttpVersionPolicy _defaultVersionPolicy = HttpUtilities.DefaultVersionPolicy;
+        private Version _defaultRequestVersion = HttpRequestMessage.DefaultRequestVersion;
+        private HttpVersionPolicy _defaultVersionPolicy = HttpRequestMessage.DefaultVersionPolicy;
 
         private Uri? _baseAddress;
         private TimeSpan _timeout;
@@ -78,7 +78,12 @@ public Uri? BaseAddress
             get => _baseAddress;
             set
             {
-                CheckBaseAddress(value, nameof(value));
+                // It's OK to not have a base address specified, but if one is, it needs to be absolute.
+                if (value is not null && !value.IsAbsoluteUri)
+                {
+                    throw new ArgumentException(SR.net_http_client_absolute_baseaddress_required, nameof(value));
+                }
+
                 CheckDisposedOrStarted();
 
                 if (NetEventSource.Log.IsEnabled()) NetEventSource.UriBaseAddress(this, value);
@@ -621,7 +626,7 @@ private void HandleFailure(Exception e, bool telemetryStarted, HttpResponseMessa
 
         private static bool StartSend(HttpRequestMessage request)
         {
-            if (HttpTelemetry.Log.IsEnabled() && request.RequestUri != null)
+            if (HttpTelemetry.Log.IsEnabled())
             {
                 HttpTelemetry.Log.RequestStart(request);
                 return true;
@@ -810,24 +815,6 @@ private void PrepareRequestMessage(HttpRequestMessage request)
             return (pendingRequestsCts, DisposeTokenSource: false, pendingRequestsCts);
         }
 
-        private static void CheckBaseAddress(Uri? baseAddress, string parameterName)
-        {
-            if (baseAddress == null)
-            {
-                return; // It's OK to not have a base address specified.
-            }
-
-            if (!baseAddress.IsAbsoluteUri)
-            {
-                throw new ArgumentException(SR.net_http_client_absolute_baseaddress_required, parameterName);
-            }
-
-            if (!HttpUtilities.IsHttpUri(baseAddress))
-            {
-                throw new ArgumentException(HttpUtilities.InvalidUriMessage, parameterName);
-            }
-        }
-
         private static bool IsNativeHandlerEnabled()
         {
             if (!AppContext.TryGetSwitch("System.Net.Http.UseNativeHttpHandler", out bool isEnabled))

src/libraries/System.Net.Http/src/System/Net/Http/HttpMessageInvoker.cs

@@ -34,16 +34,15 @@ public HttpMessageInvoker(HttpMessageHandler handler, bool disposeHandler)
         }
 
         [UnsupportedOSPlatformAttribute("browser")]
-        public virtual HttpResponseMessage Send(HttpRequestMessage request,
-            CancellationToken cancellationToken)
+        public virtual HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
         {
             if (request == null)
             {
                 throw new ArgumentNullException(nameof(request));
             }
             CheckDisposed();
 
-            if (HttpTelemetry.Log.IsEnabled() && !request.WasSentByHttpClient() && request.RequestUri != null)
+            if (ShouldSendWithTelemetry(request))
             {
                 HttpTelemetry.Log.RequestStart(request);
 
@@ -67,16 +66,15 @@ public virtual HttpResponseMessage Send(HttpRequestMessage request,
             }
         }
 
-        public virtual Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
-            CancellationToken cancellationToken)
+        public virtual Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
             if (request == null)
             {
                 throw new ArgumentNullException(nameof(request));
             }
             CheckDisposed();
 
-            if (HttpTelemetry.Log.IsEnabled() && !request.WasSentByHttpClient() && request.RequestUri != null)
+            if (ShouldSendWithTelemetry(request))
             {
                 return SendAsyncWithTelemetry(_handler, request, cancellationToken);
             }
@@ -103,6 +101,12 @@ static async Task<HttpResponseMessage> SendAsyncWithTelemetry(HttpMessageHandler
             }
         }
 
+        private static bool ShouldSendWithTelemetry(HttpRequestMessage request) =>
+            HttpTelemetry.Log.IsEnabled() &&
+            !request.WasSentByHttpClient() &&
+            request.RequestUri is Uri requestUri &&
+            requestUri.IsAbsoluteUri;
+
         internal static bool LogRequestFailed(bool telemetryStarted)
         {
             if (HttpTelemetry.Log.IsEnabled() && telemetryStarted)

src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs

@@ -1,17 +1,18 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics.CodeAnalysis;
 using System.Net.Http.Headers;
 using System.Text;
 using System.Threading;
 using System.Collections.Generic;
-using System.Diagnostics;
 
 namespace System.Net.Http
 {
     public class HttpRequestMessage : IDisposable
     {
+        internal static Version DefaultRequestVersion => HttpVersion.Version11;
+        internal static HttpVersionPolicy DefaultVersionPolicy => HttpVersionPolicy.RequestVersionOrLower;
+
         private const int MessageNotYetSent = 0;
         private const int MessageAlreadySent = 1;
 
@@ -101,29 +102,12 @@ public Uri? RequestUri
             get { return _requestUri; }
             set
             {
-                if ((value != null) && (value.IsAbsoluteUri) && (!HttpUtilities.IsHttpUri(value)))
-                {
-                    throw new ArgumentException(HttpUtilities.InvalidUriMessage, nameof(value));
-                }
                 CheckDisposed();
-
-                // It's OK to set 'null'. HttpClient will add the 'BaseAddress'. If there is no 'BaseAddress'
-                // sending this message will throw.
                 _requestUri = value;
             }
         }
 
-        public HttpRequestHeaders Headers
-        {
-            get
-            {
-                if (_headers == null)
-                {
-                    _headers = new HttpRequestHeaders();
-                }
-                return _headers;
-            }
-        }
+        public HttpRequestHeaders Headers => _headers ??= new HttpRequestHeaders();
 
         internal bool HasHeaders => _headers != null;
 
@@ -139,22 +123,18 @@ public HttpRequestMessage()
 
         public HttpRequestMessage(HttpMethod method, Uri? requestUri)
         {
-            InitializeValues(method, requestUri);
+            // It's OK to have a 'null' request Uri. If HttpClient is used, the 'BaseAddress' will be added.
+            // If there is no 'BaseAddress', sending this request message will throw.
+            // Note that we also allow the string to be empty: null and empty are considered equivalent.
+            _method = method ?? throw new ArgumentNullException(nameof(method));
+            _requestUri = requestUri;
+            _version = DefaultRequestVersion;
+            _versionPolicy = DefaultVersionPolicy;
         }
 
         public HttpRequestMessage(HttpMethod method, string? requestUri)
+            : this(method, string.IsNullOrEmpty(requestUri) ? null : new Uri(requestUri, UriKind.RelativeOrAbsolute))
         {
-            // It's OK to have a 'null' request Uri. If HttpClient is used, the 'BaseAddress' will be added.
-            // If there is no 'BaseAddress', sending this request message will throw.
-            // Note that we also allow the string to be empty: null and empty are considered equivalent.
-            if (string.IsNullOrEmpty(requestUri))
-            {
-                InitializeValues(method, null);
-            }
-            else
-            {
-                InitializeValues(method, new Uri(requestUri, UriKind.RelativeOrAbsolute));
-            }
         }
 
         public override string ToString()
@@ -179,25 +159,6 @@ public override string ToString()
             return sb.ToString();
         }
 
-        [MemberNotNull(nameof(_method))]
-        [MemberNotNull(nameof(_version))]
-        private void InitializeValues(HttpMethod method, Uri? requestUri)
-        {
-            if (method is null)
-            {
-                throw new ArgumentNullException(nameof(method));
-            }
-            if ((requestUri != null) && (requestUri.IsAbsoluteUri) && (!HttpUtilities.IsHttpUri(requestUri)))
-            {
-                throw new ArgumentException(HttpUtilities.InvalidUriMessage, nameof(requestUri));
-            }
-
-            _method = method;
-            _requestUri = requestUri;
-            _version = HttpUtilities.DefaultRequestVersion;
-            _versionPolicy = HttpUtilities.DefaultVersionPolicy;
-        }
-
         internal bool MarkAsSent()
         {
             return Interlocked.Exchange(ref _sendStatus, MessageAlreadySent) == MessageNotYetSent;

src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs

@@ -10,7 +10,8 @@ namespace System.Net.Http
 {
     public class HttpResponseMessage : IDisposable
     {
-        private const HttpStatusCode defaultStatusCode = HttpStatusCode.OK;
+        private const HttpStatusCode DefaultStatusCode = HttpStatusCode.OK;
+        private static Version DefaultResponseVersion => HttpVersion.Version11;
 
         private HttpStatusCode _statusCode;
         private HttpResponseHeaders? _headers;
@@ -149,7 +150,7 @@ public bool IsSuccessStatusCode
         }
 
         public HttpResponseMessage()
-            : this(defaultStatusCode)
+            : this(DefaultStatusCode)
         {
         }
 
@@ -161,7 +162,7 @@ public HttpResponseMessage(HttpStatusCode statusCode)
             }
 
             _statusCode = statusCode;
-            _version = HttpUtilities.DefaultResponseVersion;
+            _version = DefaultResponseVersion;
         }
 
         public HttpResponseMessage EnsureSuccessStatusCode()

src/libraries/System.Net.Http/src/System/Net/Http/HttpTelemetry.cs

@@ -36,7 +36,7 @@ private void RequestStart(string scheme, string host, int port, string pathAndQu
         [NonEvent]
         public void RequestStart(HttpRequestMessage request)
         {
-            Debug.Assert(request.RequestUri != null);
+            Debug.Assert(request.RequestUri != null && request.RequestUri.IsAbsoluteUri);
 
             RequestStart(
                 request.RequestUri.Scheme,