Skip to content

Commit 9da4d07

Browse files
MihaZupanMihaZupan
authored
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
1 parent a54e25b commit 9da4d07

File tree

20 files changed

+218
-188
lines changed

20 files changed

+218
-188
lines changed

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1950,5 +1950,23 @@ public async Task GetAsync_InvalidUrl_ExpectedExceptionThrown()
19501950
await Assert.ThrowsAsync<HttpRequestException>(() => client.GetStringAsync(invalidUri));
19511951
}
19521952
}
1953+
1954+
// HttpRequestMessage ctor guards against such Uris before .NET 6. We allow passing relative/unknown Uris to BrowserHttpHandler.
1955+
public static bool InvalidRequestUriTest_IsSupported => PlatformDetection.IsNotNetFramework && PlatformDetection.IsNotBrowser;
1956+
1957+
[ConditionalFact(nameof(InvalidRequestUriTest_IsSupported))]
1958+
public async Task SendAsync_InvalidRequestUri_Throws()
1959+
{
1960+
using var invoker = new HttpMessageInvoker(CreateHttpClientHandler());
1961+
1962+
var request = new HttpRequestMessage(HttpMethod.Get, (Uri)null);
1963+
await Assert.ThrowsAsync<InvalidOperationException>(() => invoker.SendAsync(request, CancellationToken.None));
1964+
1965+
request = new HttpRequestMessage(HttpMethod.Get, new Uri("/relative", UriKind.Relative));
1966+
await Assert.ThrowsAsync<InvalidOperationException>(() => invoker.SendAsync(request, CancellationToken.None));
1967+
1968+
request = new HttpRequestMessage(HttpMethod.Get, new Uri("foo://foo.bar"));
1969+
await Assert.ThrowsAsync<NotSupportedException>(() => invoker.SendAsync(request, CancellationToken.None));
1970+
}
19531971
}
19541972
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,4 +126,10 @@
126126
<data name="PlatformNotSupported_WinHttpHandler" xml:space="preserve">
127127
<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>
128128
</data>
129+
<data name="net_http_unsupported_requesturi_scheme" xml:space="preserve">
130+
<value>The '{0}' scheme is not supported.</value>
131+
</data>
132+
<data name="net_http_client_invalid_requesturi" xml:space="preserve">
133+
<value>An invalid request URI was provided. Either the request URI must be an absolute URI or BaseAddress must be set.</value>
134+
</data>
129135
</root>

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,17 @@ protected override Task<HttpResponseMessage> SendAsync(
578578
throw new ArgumentNullException(nameof(request), SR.net_http_handler_norequest);
579579
}
580580

581+
Uri? requestUri = request.RequestUri;
582+
if (requestUri is null || !requestUri.IsAbsoluteUri)
583+
{
584+
throw new InvalidOperationException(SR.net_http_client_invalid_requesturi);
585+
}
586+
587+
if (requestUri.Scheme != Uri.UriSchemeHttp && requestUri.Scheme != Uri.UriSchemeHttps)
588+
{
589+
throw new NotSupportedException(SR.Format(SR.net_http_unsupported_requesturi_scheme, requestUri.Scheme));
590+
}
591+
581592
// Check for invalid combinations of properties.
582593
if (_proxy != null && _windowsProxyUsePolicy != WindowsProxyUsePolicy.UseCustomProxy)
583594
{

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,13 +196,10 @@
196196
<value>The base address must be an absolute URI.</value>
197197
</data>
198198
<data name="net_http_client_invalid_requesturi" xml:space="preserve">
199-
<value>An invalid request URI was provided. The request URI must either be an absolute URI or BaseAddress must be set.</value>
199+
<value>An invalid request URI was provided. Either the request URI must be an absolute URI or BaseAddress must be set.</value>
200200
</data>
201-
<data name="net_http_client_http_baseaddress_required" xml:space="preserve">
202-
<value>Only 'http' and 'https' schemes are allowed.</value>
203-
</data>
204-
<data name="net_http_client_http_browser_baseaddress_required" xml:space="preserve">
205-
<value>Only 'http', 'https', and 'blob' schemes are allowed.</value>
201+
<data name="net_http_unsupported_requesturi_scheme" xml:space="preserve">
202+
<value>The '{0}' scheme is not supported.</value>
206203
</data>
207204
<data name="net_http_parser_invalid_base64_string" xml:space="preserve">
208205
<value>Value '{0}' is not a valid Base64 string. Error: {1}</value>

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
<Compile Include="System\Net\Http\HttpResponseMessage.cs" />
5454
<Compile Include="System\Net\Http\HttpRuleParser.cs" />
5555
<Compile Include="System\Net\Http\HttpTelemetry.cs" />
56-
<Compile Include="System\Net\Http\HttpUtilities.cs" />
5756
<Compile Include="System\Net\Http\HttpVersionPolicy.cs" />
5857
<Compile Include="System\Net\Http\MessageProcessingHandler.cs" />
5958
<Compile Include="System\Net\Http\MultipartContent.cs" />
@@ -180,6 +179,7 @@
180179
<Compile Include="System\Net\Http\SocketsHttpHandler\HttpContentStream.cs" />
181180
<Compile Include="System\Net\Http\SocketsHttpHandler\HttpContentWriteStream.cs" />
182181
<Compile Include="System\Net\Http\SocketsHttpHandler\HttpKeepAlivePingPolicy.cs" />
182+
<Compile Include="System\Net\Http\SocketsHttpHandler\HttpUtilities.cs" />
183183
<Compile Include="System\Net\Http\SocketsHttpHandler\IHttpTrace.cs" />
184184
<Compile Include="System\Net\Http\SocketsHttpHandler\IMultiWebProxy.cs" />
185185
<Compile Include="System\Net\Http\SocketsHttpHandler\MultiProxy.cs" />
@@ -188,7 +188,6 @@
188188
<Compile Include="System\Net\Http\SocketsHttpHandler\SocketsHttpConnectionContext.cs" />
189189
<Compile Include="System\Net\Http\SocketsHttpHandler\SocketsHttpHandler.cs" />
190190
<Compile Include="System\Net\Http\HttpTelemetry.AnyOS.cs" />
191-
<Compile Include="System\Net\Http\HttpUtilities.AnyOS.cs" />
192191
<Compile Include="System\Net\Http\SocketsHttpHandler\SystemProxyInfo.cs" />
193192
<Compile Include="System\Net\Http\SocketsHttpHandler\SocksHelper.cs" />
194193
<Compile Include="System\Net\Http\SocketsHttpHandler\SocksException.cs" />
@@ -623,7 +622,6 @@
623622
<Compile Include="System\Net\Http\BrowserHttpHandler\SocketsHttpHandler.cs" />
624623
<Compile Include="System\Net\Http\BrowserHttpHandler\BrowserHttpHandler.cs" />
625624
<Compile Include="System\Net\Http\BrowserHttpHandler\HttpTelemetry.Browser.cs" />
626-
<Compile Include="System\Net\Http\BrowserHttpHandler\HttpUtilities.Browser.cs" />
627625
<Compile Include="$(CommonPath)System\Net\Http\HttpHandlerDefaults.cs"
628626
Link="Common\System\Net\Http\HttpHandlerDefaults.cs" />
629627
</ItemGroup>

‎src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/HttpUtilities.Browser.cs

Lines changed: 0 additions & 22 deletions
This file was deleted.

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

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ public partial class HttpClient : HttpMessageInvoker
2626

2727
private CancellationTokenSource _pendingRequestsCts;
2828
private HttpRequestHeaders? _defaultRequestHeaders;
29-
private Version _defaultRequestVersion = HttpUtilities.DefaultRequestVersion;
30-
private HttpVersionPolicy _defaultVersionPolicy = HttpUtilities.DefaultVersionPolicy;
29+
private Version _defaultRequestVersion = HttpRequestMessage.DefaultRequestVersion;
30+
private HttpVersionPolicy _defaultVersionPolicy = HttpRequestMessage.DefaultVersionPolicy;
3131

3232
private Uri? _baseAddress;
3333
private TimeSpan _timeout;
@@ -78,7 +78,12 @@ public Uri? BaseAddress
7878
get => _baseAddress;
7979
set
8080
{
81-
CheckBaseAddress(value, nameof(value));
81+
// It's OK to not have a base address specified, but if one is, it needs to be absolute.
82+
if (value is not null && !value.IsAbsoluteUri)
83+
{
84+
throw new ArgumentException(SR.net_http_client_absolute_baseaddress_required, nameof(value));
85+
}
86+
8287
CheckDisposedOrStarted();
8388

8489
if (NetEventSource.Log.IsEnabled()) NetEventSource.UriBaseAddress(this, value);
@@ -621,7 +626,7 @@ private void HandleFailure(Exception e, bool telemetryStarted, HttpResponseMessa
621626

622627
private static bool StartSend(HttpRequestMessage request)
623628
{
624-
if (HttpTelemetry.Log.IsEnabled() && request.RequestUri != null)
629+
if (HttpTelemetry.Log.IsEnabled())
625630
{
626631
HttpTelemetry.Log.RequestStart(request);
627632
return true;
@@ -810,24 +815,6 @@ private void PrepareRequestMessage(HttpRequestMessage request)
810815
return (pendingRequestsCts, DisposeTokenSource: false, pendingRequestsCts);
811816
}
812817

813-
private static void CheckBaseAddress(Uri? baseAddress, string parameterName)
814-
{
815-
if (baseAddress == null)
816-
{
817-
return; // It's OK to not have a base address specified.
818-
}
819-
820-
if (!baseAddress.IsAbsoluteUri)
821-
{
822-
throw new ArgumentException(SR.net_http_client_absolute_baseaddress_required, parameterName);
823-
}
824-
825-
if (!HttpUtilities.IsHttpUri(baseAddress))
826-
{
827-
throw new ArgumentException(HttpUtilities.InvalidUriMessage, parameterName);
828-
}
829-
}
830-
831818
private static bool IsNativeHandlerEnabled()
832819
{
833820
if (!AppContext.TryGetSwitch("System.Net.Http.UseNativeHttpHandler", out bool isEnabled))

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,15 @@ public HttpMessageInvoker(HttpMessageHandler handler, bool disposeHandler)
3434
}
3535

3636
[UnsupportedOSPlatformAttribute("browser")]
37-
public virtual HttpResponseMessage Send(HttpRequestMessage request,
38-
CancellationToken cancellationToken)
37+
public virtual HttpResponseMessage Send(HttpRequestMessage request, CancellationToken cancellationToken)
3938
{
4039
if (request == null)
4140
{
4241
throw new ArgumentNullException(nameof(request));
4342
}
4443
CheckDisposed();
4544

46-
if (HttpTelemetry.Log.IsEnabled() && !request.WasSentByHttpClient() && request.RequestUri != null)
45+
if (ShouldSendWithTelemetry(request))
4746
{
4847
HttpTelemetry.Log.RequestStart(request);
4948

@@ -67,16 +66,15 @@ public virtual HttpResponseMessage Send(HttpRequestMessage request,
6766
}
6867
}
6968

70-
public virtual Task<HttpResponseMessage> SendAsync(HttpRequestMessage request,
71-
CancellationToken cancellationToken)
69+
public virtual Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
7270
{
7371
if (request == null)
7472
{
7573
throw new ArgumentNullException(nameof(request));
7674
}
7775
CheckDisposed();
7876

79-
if (HttpTelemetry.Log.IsEnabled() && !request.WasSentByHttpClient() && request.RequestUri != null)
77+
if (ShouldSendWithTelemetry(request))
8078
{
8179
return SendAsyncWithTelemetry(_handler, request, cancellationToken);
8280
}
@@ -103,6 +101,12 @@ static async Task<HttpResponseMessage> SendAsyncWithTelemetry(HttpMessageHandler
103101
}
104102
}
105103

104+
private static bool ShouldSendWithTelemetry(HttpRequestMessage request) =>
105+
HttpTelemetry.Log.IsEnabled() &&
106+
!request.WasSentByHttpClient() &&
107+
request.RequestUri is Uri requestUri &&
108+
requestUri.IsAbsoluteUri;
109+
106110
internal static bool LogRequestFailed(bool telemetryStarted)
107111
{
108112
if (HttpTelemetry.Log.IsEnabled() && telemetryStarted)

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

Lines changed: 12 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Diagnostics.CodeAnalysis;
54
using System.Net.Http.Headers;
65
using System.Text;
76
using System.Threading;
87
using System.Collections.Generic;
9-
using System.Diagnostics;
108

119
namespace System.Net.Http
1210
{
1311
public class HttpRequestMessage : IDisposable
1412
{
13+
internal static Version DefaultRequestVersion => HttpVersion.Version11;
14+
internal static HttpVersionPolicy DefaultVersionPolicy => HttpVersionPolicy.RequestVersionOrLower;
15+
1516
private const int MessageNotYetSent = 0;
1617
private const int MessageAlreadySent = 1;
1718

@@ -101,29 +102,12 @@ public Uri? RequestUri
101102
get { return _requestUri; }
102103
set
103104
{
104-
if ((value != null) && (value.IsAbsoluteUri) && (!HttpUtilities.IsHttpUri(value)))
105-
{
106-
throw new ArgumentException(HttpUtilities.InvalidUriMessage, nameof(value));
107-
}
108105
CheckDisposed();
109-
110-
// It's OK to set 'null'. HttpClient will add the 'BaseAddress'. If there is no 'BaseAddress'
111-
// sending this message will throw.
112106
_requestUri = value;
113107
}
114108
}
115109

116-
public HttpRequestHeaders Headers
117-
{
118-
get
119-
{
120-
if (_headers == null)
121-
{
122-
_headers = new HttpRequestHeaders();
123-
}
124-
return _headers;
125-
}
126-
}
110+
public HttpRequestHeaders Headers => _headers ??= new HttpRequestHeaders();
127111

128112
internal bool HasHeaders => _headers != null;
129113

@@ -139,22 +123,18 @@ public HttpRequestMessage()
139123

140124
public HttpRequestMessage(HttpMethod method, Uri? requestUri)
141125
{
142-
InitializeValues(method, requestUri);
126+
// It's OK to have a 'null' request Uri. If HttpClient is used, the 'BaseAddress' will be added.
127+
// If there is no 'BaseAddress', sending this request message will throw.
128+
// Note that we also allow the string to be empty: null and empty are considered equivalent.
129+
_method = method ?? throw new ArgumentNullException(nameof(method));
130+
_requestUri = requestUri;
131+
_version = DefaultRequestVersion;
132+
_versionPolicy = DefaultVersionPolicy;
143133
}
144134

145135
public HttpRequestMessage(HttpMethod method, string? requestUri)
136+
: this(method, string.IsNullOrEmpty(requestUri) ? null : new Uri(requestUri, UriKind.RelativeOrAbsolute))
146137
{
147-
// It's OK to have a 'null' request Uri. If HttpClient is used, the 'BaseAddress' will be added.
148-
// If there is no 'BaseAddress', sending this request message will throw.
149-
// Note that we also allow the string to be empty: null and empty are considered equivalent.
150-
if (string.IsNullOrEmpty(requestUri))
151-
{
152-
InitializeValues(method, null);
153-
}
154-
else
155-
{
156-
InitializeValues(method, new Uri(requestUri, UriKind.RelativeOrAbsolute));
157-
}
158138
}
159139

160140
public override string ToString()
@@ -179,25 +159,6 @@ public override string ToString()
179159
return sb.ToString();
180160
}
181161

182-
[MemberNotNull(nameof(_method))]
183-
[MemberNotNull(nameof(_version))]
184-
private void InitializeValues(HttpMethod method, Uri? requestUri)
185-
{
186-
if (method is null)
187-
{
188-
throw new ArgumentNullException(nameof(method));
189-
}
190-
if ((requestUri != null) && (requestUri.IsAbsoluteUri) && (!HttpUtilities.IsHttpUri(requestUri)))
191-
{
192-
throw new ArgumentException(HttpUtilities.InvalidUriMessage, nameof(requestUri));
193-
}
194-
195-
_method = method;
196-
_requestUri = requestUri;
197-
_version = HttpUtilities.DefaultRequestVersion;
198-
_versionPolicy = HttpUtilities.DefaultVersionPolicy;
199-
}
200-
201162
internal bool MarkAsSent()
202163
{
203164
return Interlocked.Exchange(ref _sendStatus, MessageAlreadySent) == MessageNotYetSent;

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ namespace System.Net.Http
1010
{
1111
public class HttpResponseMessage : IDisposable
1212
{
13-
private const HttpStatusCode defaultStatusCode = HttpStatusCode.OK;
13+
private const HttpStatusCode DefaultStatusCode = HttpStatusCode.OK;
14+
private static Version DefaultResponseVersion => HttpVersion.Version11;
1415

1516
private HttpStatusCode _statusCode;
1617
private HttpResponseHeaders? _headers;
@@ -149,7 +150,7 @@ public bool IsSuccessStatusCode
149150
}
150151

151152
public HttpResponseMessage()
152-
: this(defaultStatusCode)
153+
: this(DefaultStatusCode)
153154
{
154155
}
155156

@@ -161,7 +162,7 @@ public HttpResponseMessage(HttpStatusCode statusCode)
161162
}
162163

163164
_statusCode = statusCode;
164-
_version = HttpUtilities.DefaultResponseVersion;
165+
_version = DefaultResponseVersion;
165166
}
166167

167168
public HttpResponseMessage EnsureSuccessStatusCode()

0 commit comments

Comments
 (0)