From f98796aa4e1e89f27175b2bcd64a4be9a774fdcb Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 7 Sep 2020 19:57:12 +0200 Subject: [PATCH 01/15] Implement AllowNonAsciiHeaders option + tests --- .../SocketsHttpHandler/HPack/HPackEncoder.cs | 22 +++- .../Http/SocketsHttpHandler/HttpConnection.cs | 19 ++- .../HttpConnectionSettings.cs | 25 ++++ .../HttpClientHandlerTest.Headers.cs | 110 +++++++++++++++++- .../System.Net.Http.Unit.Tests.csproj | 3 + 5 files changed, 169 insertions(+), 10 deletions(-) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs index ccf80aaa8847..39e8cda70311 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs @@ -217,16 +217,28 @@ private static bool EncodeStringLiteralValue(string value, Span destinatio { if (value.Length <= destination.Length) { - for (int i = 0; i < value.Length; i++) + if (HttpConnectionSettings.AllowNonAsciiHeaders) { - char c = value[i]; - if ((c & 0xFF80) != 0) + for (int i = 0; i < value.Length; i++) { - throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); + char c = value[i]; + destination[i] = (byte)c; } + } + else + { + for (int i = 0; i < value.Length; i++) + { + char c = value[i]; + if ((c & 0xFF80) != 0) + { + throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); + } - destination[i] = (byte)c; + destination[i] = (byte)c; + } } + bytesWritten = value.Length; return true; diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 73c9d95b952e..5cb69b97dd06 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -1147,14 +1147,25 @@ private Task WriteStringAsync(string s) if (s.Length <= _writeBuffer.Length - offset) { byte[] writeBuffer = _writeBuffer; - foreach (char c in s) + if (HttpConnectionSettings.AllowNonAsciiHeaders) { - if ((c & 0xFF80) != 0) + foreach (char c in s) { - throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); + writeBuffer[offset++] = (byte)c; + } + } + else + { + foreach (char c in s) + { + if ((c & 0xFF80) != 0) + { + throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); + } + writeBuffer[offset++] = (byte)c; } - writeBuffer[offset++] = (byte)c; } + _writeOffset = offset; return Task.CompletedTask; } diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index f6788731ce8c..2e86271e275b 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -12,9 +12,17 @@ internal sealed class HttpConnectionSettings { private const string Http2SupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP2SUPPORT"; private const string Http2SupportAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.Http2Support"; + private const string Http2UnencryptedSupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP2UNENCRYPTEDSUPPORT"; private const string Http2UnencryptedSupportAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport"; + private const string AllowNonAsciiCharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS"; + private const string AllowNonAsciiCharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders"; + + private static readonly Lazy s_allowNonAsciiHeaders = new Lazy(GetAllowNonAsciiCharactersSetting); + + internal static bool AllowNonAsciiHeaders => s_allowNonAsciiHeaders.Value; + internal DecompressionMethods _automaticDecompression = HttpHandlerDefaults.DefaultAutomaticDecompression; internal bool _useCookies = HttpHandlerDefaults.DefaultUseCookies; @@ -142,5 +150,22 @@ private static bool AllowUnencryptedHttp2 return false; } } + + private static bool GetAllowNonAsciiCharactersSetting() + { + if (AppContext.TryGetSwitch(AllowNonAsciiCharactersAppCtxSettingName, out bool value)) + { + return value; + } + + // AppContext switch wasn't used. Check the environment variable. + string envVar = Environment.GetEnvironmentVariable(AllowNonAsciiCharactersEnvironmentVariableSettingName); + if (envVar != null && (envVar.Equals("true", StringComparison.OrdinalIgnoreCase) || envVar.Equals("1"))) + { + return true; + } + + return false; + } } } diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index 6d85a2e8f124..c0e5a75a3be8 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -7,8 +7,9 @@ using System.Linq; using System.Net.Http.Headers; using System.Net.Test.Common; +using System.Text; using System.Threading.Tasks; - +using Microsoft.DotNet.RemoteExecutor; using Xunit; using Xunit.Abstractions; @@ -286,5 +287,112 @@ await server.HandleRequestAsync(headers: new[] }); }); } + + private static readonly (string Name, string[] Values)[] s_nonAsciiHeaders = new[] + { + ("Some-Header1", new[] { "\uD83D\uDE03", "UTF8-best-fit-to-latin1" }), + ("Some-Header2", new[] { "\u00FF", "\u00C4nd", "Ascii\u00A9" }), + }; + + private static void AllowNonAsciiHeaders(string useAppCtxSwitchInner) + { + if (bool.Parse(useAppCtxSwitchInner)) + { + AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders", true); + } + else + { + Environment.SetEnvironmentVariable("DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS", "True"); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void SendAsync_CustomRequestEncodingSelector_CanSendLatin1HeaderValues(bool useAppCtxSwitchOuter) + { + RemoteExecutor.Invoke((useAppCtxSwitchInner) => + { + AllowNonAsciiHeaders(useAppCtxSwitchInner); + + LoopbackServerFactory.CreateClientAndServerAsync( + async uri => + { + var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri); + + foreach ((string name, string[] values) in s_nonAsciiHeaders) + { + requestMessage.Headers.Add(name, values); + } + + using HttpClientHandler handler = CreateHttpClientHandler(); + var underlyingHandler = (SocketsHttpHandler)GetUnderlyingSocketsHttpHandler(handler); + + using HttpClient client = CreateHttpClient(handler); + + await client.SendAsync(requestMessage); + }, + async server => + { + HttpRequestData requestData = await server.HandleRequestAsync(); + + Assert.All(requestData.Headers, + h => Assert.False(h.HuffmanEncoded, "Expose raw decoded bytes once HuffmanEncoding is supported")); + + Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); + + foreach ((string name, string[] values) in s_nonAsciiHeaders) + { + byte[] valueBytes = valueEncoding.GetBytes(string.Join(", ", values)); + Assert.Single(requestData.Headers, + h => h.Name.Equals(name, StringComparison.OrdinalIgnoreCase) && h.Raw.AsSpan().IndexOf(valueBytes) != -1); + } + }) + .GetAwaiter().GetResult(); + }, useAppCtxSwitchOuter.ToString()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void SendAsync_CustomResponseEncodingSelector_CanReceiveNonAsciiHeaderValues(bool useAppCtxSwitchOuter) + { + RemoteExecutor.Invoke((useAppCtxSwitchInner) => + { + AllowNonAsciiHeaders(useAppCtxSwitchInner); + + LoopbackServerFactory.CreateClientAndServerAsync( + async uri => + { + var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri); + + using HttpClientHandler handler = CreateHttpClientHandler(); + var underlyingHandler = (SocketsHttpHandler)GetUnderlyingSocketsHttpHandler(handler); + + using HttpClient client = CreateHttpClient(handler); + + using HttpResponseMessage response = await client.SendAsync(requestMessage); + Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); + + foreach ((string name, string[] values) in s_nonAsciiHeaders) + { + IEnumerable receivedValues = Assert.Single(response.Headers, h => h.Key.Equals(name, StringComparison.OrdinalIgnoreCase)).Value; + string value = Assert.Single(receivedValues); + + string expected = valueEncoding.GetString(valueEncoding.GetBytes(string.Join(", ", values))); + Assert.Equal(expected, value, StringComparer.OrdinalIgnoreCase); + } + }, + async server => + { + List headerData = s_nonAsciiHeaders + .Select(h => new HttpHeaderData(h.Name, string.Join(", ", h.Values))) + .ToList(); + + await server.HandleRequestAsync(headers: headerData); + }) + .GetAwaiter().GetResult(); + }, useAppCtxSwitchOuter.ToString()); + } } } diff --git a/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj index b0c14d13f103..a33a612c000c 100644 --- a/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj +++ b/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj @@ -300,6 +300,9 @@ ProductionCode\System\Net\Http\SocketsHttpHandler\ArrayBuffer.cs + + ProductionCode\System\Net\Http\SocketsHttpHandler\HttpConnectionSettings.cs + ProductionCode\System\Net\Http\SocketsHttpHandler\HttpEnvironmentProxy.cs From 4753b19e8b39aff01e3ffe9d30e659dda97ef8c0 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 7 Sep 2020 20:12:15 +0200 Subject: [PATCH 02/15] add comments --- .../Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index 2e86271e275b..c955a502fb1b 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -21,6 +21,11 @@ internal sealed class HttpConnectionSettings private static readonly Lazy s_allowNonAsciiHeaders = new Lazy(GetAllowNonAsciiCharactersSetting); + // Disables a validation that checks whether Http headers contain a non-ASCII character. + // This is a workaround that has been introduced as a patch specific to the 3.1 branch. + // Unlike other options in HttpConnectionSettings, this one has a global scope. + // Lazy initialization is being used to make sure clients can also use AppContext.SetSwitch() or Environment.SetEnvironmentVariable() + // before the first calls to HttpClient API-s. internal static bool AllowNonAsciiHeaders => s_allowNonAsciiHeaders.Value; internal DecompressionMethods _automaticDecompression = HttpHandlerDefaults.DefaultAutomaticDecompression; @@ -153,6 +158,7 @@ private static bool AllowUnencryptedHttp2 private static bool GetAllowNonAsciiCharactersSetting() { + // First check for the AppContext switch, giving it priority over the environment variable. if (AppContext.TryGetSwitch(AllowNonAsciiCharactersAppCtxSettingName, out bool value)) { return value; From 16278ad634efd87a60f72267fbc902a8b1ece789 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 11:01:19 +0200 Subject: [PATCH 03/15] Move AllowNonAsciiHeaders to new class to fix build --- .../src/System.Net.Http.csproj | 1 + .../SocketsHttpHandler/HPack/HPackEncoder.cs | 2 +- .../Http/SocketsHttpHandler/HttpConnection.cs | 2 +- .../HttpConnectionSettings.cs | 31 --------------- .../SocketsHttpHandler/StaticHttpSettings.cs | 39 +++++++++++++++++++ .../System.Net.Http.Unit.Tests.csproj | 6 +-- 6 files changed, 45 insertions(+), 36 deletions(-) create mode 100644 src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs diff --git a/src/System.Net.Http/src/System.Net.Http.csproj b/src/System.Net.Http/src/System.Net.Http.csproj index fcbf55bbe1e9..8bdb3fb3e875 100644 --- a/src/System.Net.Http/src/System.Net.Http.csproj +++ b/src/System.Net.Http/src/System.Net.Http.csproj @@ -161,6 +161,7 @@ + diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs index 39e8cda70311..7bf745df4e85 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs @@ -217,7 +217,7 @@ private static bool EncodeStringLiteralValue(string value, Span destinatio { if (value.Length <= destination.Length) { - if (HttpConnectionSettings.AllowNonAsciiHeaders) + if (StaticHttpSettings.AllowNonAsciiHeaders) { for (int i = 0; i < value.Length; i++) { diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 5cb69b97dd06..50a2cea9bbea 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -1147,7 +1147,7 @@ private Task WriteStringAsync(string s) if (s.Length <= _writeBuffer.Length - offset) { byte[] writeBuffer = _writeBuffer; - if (HttpConnectionSettings.AllowNonAsciiHeaders) + if (StaticHttpSettings.AllowNonAsciiHeaders) { foreach (char c in s) { diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs index c955a502fb1b..f6788731ce8c 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionSettings.cs @@ -12,22 +12,9 @@ internal sealed class HttpConnectionSettings { private const string Http2SupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP2SUPPORT"; private const string Http2SupportAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.Http2Support"; - private const string Http2UnencryptedSupportEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_HTTP2UNENCRYPTEDSUPPORT"; private const string Http2UnencryptedSupportAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport"; - private const string AllowNonAsciiCharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS"; - private const string AllowNonAsciiCharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders"; - - private static readonly Lazy s_allowNonAsciiHeaders = new Lazy(GetAllowNonAsciiCharactersSetting); - - // Disables a validation that checks whether Http headers contain a non-ASCII character. - // This is a workaround that has been introduced as a patch specific to the 3.1 branch. - // Unlike other options in HttpConnectionSettings, this one has a global scope. - // Lazy initialization is being used to make sure clients can also use AppContext.SetSwitch() or Environment.SetEnvironmentVariable() - // before the first calls to HttpClient API-s. - internal static bool AllowNonAsciiHeaders => s_allowNonAsciiHeaders.Value; - internal DecompressionMethods _automaticDecompression = HttpHandlerDefaults.DefaultAutomaticDecompression; internal bool _useCookies = HttpHandlerDefaults.DefaultUseCookies; @@ -155,23 +142,5 @@ private static bool AllowUnencryptedHttp2 return false; } } - - private static bool GetAllowNonAsciiCharactersSetting() - { - // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(AllowNonAsciiCharactersAppCtxSettingName, out bool value)) - { - return value; - } - - // AppContext switch wasn't used. Check the environment variable. - string envVar = Environment.GetEnvironmentVariable(AllowNonAsciiCharactersEnvironmentVariableSettingName); - if (envVar != null && (envVar.Equals("true", StringComparison.OrdinalIgnoreCase) || envVar.Equals("1"))) - { - return true; - } - - return false; - } } } diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs new file mode 100644 index 000000000000..35633249c2b3 --- /dev/null +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs @@ -0,0 +1,39 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace System.Net.Http +{ + internal static class StaticHttpSettings + { + private const string AllowNonAsciiCharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS"; + private const string AllowNonAsciiCharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders"; + + private static readonly Lazy s_allowNonAsciiHeaders = new Lazy(GetAllowNonAsciiCharactersSetting); + + // Disables a validation that checks whether Http headers contain a non-ASCII character. + // This is a workaround that has been introduced as a patch specific to the 3.1 branch. + // Unlike options in HttpConnectionSettings, this one has a global scope. + // Lazy initialization is being used to make sure clients can also use AppContext.SetSwitch() or Environment.SetEnvironmentVariable() + // before the first calls to HttpClient API-s. + internal static bool AllowNonAsciiHeaders => s_allowNonAsciiHeaders.Value; + + private static bool GetAllowNonAsciiCharactersSetting() + { + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(AllowNonAsciiCharactersAppCtxSettingName, out bool value)) + { + return value; + } + + // AppContext switch wasn't used. Check the environment variable. + string envVar = Environment.GetEnvironmentVariable(AllowNonAsciiCharactersEnvironmentVariableSettingName); + if (envVar != null && (envVar.Equals("true", StringComparison.OrdinalIgnoreCase) || envVar.Equals("1"))) + { + return true; + } + + return false; + } + } +} diff --git a/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj b/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj index a33a612c000c..eabcf843f293 100644 --- a/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj +++ b/src/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj @@ -300,9 +300,6 @@ ProductionCode\System\Net\Http\SocketsHttpHandler\ArrayBuffer.cs - - ProductionCode\System\Net\Http\SocketsHttpHandler\HttpConnectionSettings.cs - ProductionCode\System\Net\Http\SocketsHttpHandler\HttpEnvironmentProxy.cs @@ -342,6 +339,9 @@ ProductionCode\System\Net\Http\SocketsHttpHandler\HPack\DynamicTable.cs + + ProductionCode\System\Net\Http\SocketsHttpHandler\StaticHttpSettings.cs + ProductionCode\System\Net\Http\SocketsHttpHandler\SystemProxyInfo.cs From 4a6d304f2aa137e4c17504c5cb425606fa96ef08 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 16:55:11 +0200 Subject: [PATCH 04/15] fix tests + cover non- ISO-8859 cases --- .../System/Net/Http/GenericLoopbackServer.cs | 4 +- .../tests/System/Net/Http/LoopbackServer.cs | 81 +++++++-- .../Http/SocketsHttpHandler/HttpConnection.cs | 4 +- .../HttpClientHandlerTest.Headers.cs | 172 +++++++++++------- .../FunctionalTests/SocketsHttpHandlerTest.cs | 62 +++++++ 5 files changed, 243 insertions(+), 80 deletions(-) diff --git a/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs index a1ca61dce27f..9110eefcbbab 100644 --- a/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -87,13 +87,15 @@ public struct HttpHeaderData public string Value { get; } public bool HuffmanEncoded { get; } public byte[] Raw { get; } + public bool Latin1 { get; } - public HttpHeaderData(string name, string value, bool huffmanEncoded = false, byte[] raw = null) + public HttpHeaderData(string name, string value, bool huffmanEncoded = false, byte[] raw = null, bool latin1 = false) { Name = name; Value = value; HuffmanEncoded = huffmanEncoded; Raw = raw; + Latin1 = latin1; } public override string ToString() => Name == null ? "" : (Name + ": " + (Value ?? string.Empty)); diff --git a/src/Common/tests/System/Net/Http/LoopbackServer.cs b/src/Common/tests/System/Net/Http/LoopbackServer.cs index 07f794f6d61f..129aeb3fb24e 100644 --- a/src/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/Common/tests/System/Net/Http/LoopbackServer.cs @@ -16,6 +16,9 @@ namespace System.Net.Test.Common { public sealed partial class LoopbackServer : GenericLoopbackServer, IDisposable { + private static readonly byte[] s_newLineBytes = new byte[] { (byte)'\r', (byte)'\n' }; + private static readonly byte[] s_colonSpaceBytes = new byte[] { (byte)':', (byte)' ' }; + private Socket _listenSocket; private Options _options; private Uri _uri; @@ -514,6 +517,16 @@ public string ReadLine() } public async Task ReadLineAsync() + { + byte[] lineBytes = await ReadLineBytesAsync().ConfigureAwait(false); + + if (lineBytes is null) + return null; + + return Encoding.ASCII.GetString(lineBytes); + } + + private async Task ReadLineBytesAsync() { int index = 0; int startSearch = _readStart; @@ -560,7 +573,7 @@ public async Task ReadLineAsync() if (_readBuffer[_readStart + stringLength] == '\n') { stringLength--; } if (_readBuffer[_readStart + stringLength] == '\r') { stringLength--; } - string line = System.Text.Encoding.ASCII.GetString(_readBuffer, _readStart, stringLength + 1); + byte[] line = _readBuffer.AsSpan(_readStart, stringLength + 1).ToArray(); _readStart = index + 1; return line; } @@ -568,6 +581,32 @@ public async Task ReadLineAsync() return null; } + private async Task> ReadRequestHeaderBytesAsync() + { + var lines = new List(); + + byte[] line; + + while (true) + { + line = await ReadLineBytesAsync().ConfigureAwait(false); + + if (line is null || line.Length == 0) + { + break; + } + + lines.Add(line); + } + + if (line == null) + { + throw new IOException("Unexpected EOF trying to read request header"); + } + + return lines; + } + public override void Dispose() { try @@ -640,24 +679,24 @@ public async Task> ReadRequestHeaderAndSendResponseAsync(HttpStatus public override async Task ReadRequestDataAsync(bool readBody = true) { - List headerLines = null; HttpRequestData requestData = new HttpRequestData(); - headerLines = await ReadRequestHeaderAsync().ConfigureAwait(false); + List headerLines = await ReadRequestHeaderBytesAsync().ConfigureAwait(false); // Parse method and path - string[] splits = headerLines[0].Split(' '); + string[] splits = Encoding.ASCII.GetString(headerLines[0]).Split(' '); requestData.Method = splits[0]; requestData.Path = splits[1]; // Convert header lines to key/value pairs // Skip first line since it's the status line - foreach (var line in headerLines.Skip(1)) + foreach (byte[] lineBytes in headerLines.Skip(1)) { + string line = Encoding.ASCII.GetString(lineBytes); int offset = line.IndexOf(':'); string name = line.Substring(0, offset); string value = line.Substring(offset + 1).TrimStart(); - requestData.Headers.Add(new HttpHeaderData(name, value)); + requestData.Headers.Add(new HttpHeaderData(name, value, raw: lineBytes)); } if (requestData.Method != "GET") @@ -737,7 +776,7 @@ public override async Task ReadRequestBodyAsync() public override async Task SendResponseAsync(HttpStatusCode? statusCode = HttpStatusCode.OK, IList headers = null, string content = null, bool isFinal = true, int requestId = 0) { - string headerString = null; + MemoryStream headerBytes = new MemoryStream(); int contentLength = -1; bool isChunked = false; bool hasContentLength = false; @@ -762,22 +801,36 @@ public override async Task SendResponseAsync(HttpStatusCode? statusCode = HttpSt isChunked = true; } - headerString = headerString + $"{headerData.Name}: {headerData.Value}\r\n"; + byte[] nameBytes = Encoding.ASCII.GetBytes(headerData.Name); + headerBytes.Write(nameBytes, 0, nameBytes.Length); + headerBytes.Write(s_colonSpaceBytes, 0, s_colonSpaceBytes.Length); + + byte[] valueBytes = (headerData.Latin1 ? Encoding.GetEncoding("ISO-8859-1") : Encoding.ASCII).GetBytes(headerData.Value); + headerBytes.Write(valueBytes, 0, valueBytes.Length); + headerBytes.Write(s_newLineBytes, 0, s_newLineBytes.Length); } } bool endHeaders = content != null || isFinal; if (statusCode != null) { - // If we need to send status line, prepped it to headers and possibly add missing headers to the end. - headerString = + byte[] temp = headerBytes.ToArray(); + headerBytes.SetLength(0); + byte[] headerStartBytes = Encoding.ASCII.GetBytes( $"HTTP/1.1 {(int)statusCode} {GetStatusDescription((HttpStatusCode)statusCode)}\r\n" + - (!hasContentLength && !isChunked && content != null ? $"Content-length: {content.Length}\r\n" : "") + - headerString + - (endHeaders ? "\r\n" : ""); + (!hasContentLength && !isChunked && content != null ? $"Content-length: {content.Length}\r\n" : "")); + headerBytes.Write(headerStartBytes, 0, headerStartBytes.Length); + headerBytes.Write(temp, 0, temp.Length); + + if (endHeaders) + { + headerBytes.Write(s_newLineBytes, 0, s_newLineBytes.Length); + } } - await SendResponseAsync(headerString).ConfigureAwait(false); + headerBytes.Position = 0; + await headerBytes.CopyToAsync(_stream).ConfigureAwait(false); + if (content != null) { await SendResponseBodyAsync(content, isFinal: isFinal, requestId: requestId).ConfigureAwait(false); diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 50a2cea9bbea..43d821b9ee8e 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -1200,10 +1200,12 @@ private async Task WriteStringAsyncSlow(string s) for (int i = 0; i < s.Length; i++) { char c = s[i]; - if ((c & 0xFF80) != 0) + + if (!StaticHttpSettings.AllowNonAsciiHeaders && (c & 0xFF80) != 0) { throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); } + await WriteByteAsync((byte)c).ConfigureAwait(false); } } diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index c0e5a75a3be8..08053facdd8a 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -7,6 +7,7 @@ using System.Linq; using System.Net.Http.Headers; using System.Net.Test.Common; +using System.Runtime.InteropServices; using System.Text; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; @@ -134,7 +135,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => { using (HttpClient client = CreateHttpClient()) { - HttpResponseMessage response = await client.GetAsync(uri).ConfigureAwait(false); + HttpResponseMessage response = await client.GetAsync(uri).ConfigureAwait(false); Assert.Equal(headers.Count, response.Headers.Count()); Assert.NotNull(response.Headers.GetValues("x-empty")); } @@ -152,18 +153,18 @@ await server.AcceptConnectionAsync(async connection => [Fact] public async Task GetAsync_MissingExpires_ReturnNull() { - await LoopbackServerFactory.CreateClientAndServerAsync(async uri => - { + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { using (HttpClient client = CreateHttpClient()) { HttpResponseMessage response = await client.GetAsync(uri); Assert.Null(response.Content.Headers.Expires); } }, - async server => - { - await server.HandleRequestAsync(HttpStatusCode.OK); - }); + async server => + { + await server.HandleRequestAsync(HttpStatusCode.OK); + }); } [Theory] @@ -287,94 +288,137 @@ await server.HandleRequestAsync(headers: new[] }); }); } + } + + public abstract class HttpClientHandlerTest_Headers_NonAscii : HttpClientHandlerTestBase + { + public HttpClientHandlerTest_Headers_NonAscii() : base(null) + { + } - private static readonly (string Name, string[] Values)[] s_nonAsciiHeaders = new[] - { - ("Some-Header1", new[] { "\uD83D\uDE03", "UTF8-best-fit-to-latin1" }), - ("Some-Header2", new[] { "\u00FF", "\u00C4nd", "Ascii\u00A9" }), + private static readonly (string Name, string[] Values)[] s_validLatin1Characters = new[] + { + ("a", new[] { "aa"}) + //("a", new[] { "\x0081"}) + //("Header1", new[] { "\x0081\x00FF\x00FE", "ascii-only" }), + //("Set-Cookie", new[] { "\u00FF", "\x00B0\x00B1\x00B2", "Ascii\x00B8" }), }; - private static void AllowNonAsciiHeaders(string useAppCtxSwitchInner) + private static readonly (string Name, string[] Values)[] s_invalidUnicodeCharacters = new[] { + ("header-0", new[] { "\uD83D\uDE03", "\uD83D\uDE48\uD83D\uDE49\uD83D\uDE4A" }), + ("Set-Cookie", new[] { "\uD83C\uDDF8\uD83C\uDDEE" }), + }; + + private static (string Name, string[] Values)[] GetNonAsciiTestHeaders(bool unicode) => unicode ? s_invalidUnicodeCharacters : s_validLatin1Characters; + + private static bool AllowNonAsciiHeaders(string useAppCtxSwitchInner, string switchValueInner) + { + bool switchValue = bool.Parse(switchValueInner); if (bool.Parse(useAppCtxSwitchInner)) { - AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders", true); + AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders", switchValue); } else { - Environment.SetEnvironmentVariable("DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS", "True"); + Environment.SetEnvironmentVariable("DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS", switchValueInner); } + return switchValue; } - [Theory] - [InlineData(false)] - [InlineData(true)] - public void SendAsync_CustomRequestEncodingSelector_CanSendLatin1HeaderValues(bool useAppCtxSwitchOuter) + // bool useAppCtxSwitch, bool switchValue, bool utf8 + public static readonly TheoryData NonAsciiTestConfigurations = new TheoryData() { - RemoteExecutor.Invoke((useAppCtxSwitchInner) => - { - AllowNonAsciiHeaders(useAppCtxSwitchInner); + { true, false, false }, + { true, false, true }, + { true, true, false }, + { true, true, true }, + { false, true, false }, + }; - LoopbackServerFactory.CreateClientAndServerAsync( - async uri => - { - var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri); + // Since RemoteExecutor cannot invoke delegates defined in abstract types, we should define the actual test cases in derived classes. + protected int SendAsync_SendNonAsciiCharacters_Impl(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) + { + bool allowNonAscii = AllowNonAsciiHeaders(useAppCtxSwitchInner, switchValueInner); + bool unicode = bool.Parse(unicodeInner); + bool expectSuccess = allowNonAscii && !unicode; - foreach ((string name, string[] values) in s_nonAsciiHeaders) - { - requestMessage.Headers.Add(name, values); - } + (string name, string[] values)[] headers = GetNonAsciiTestHeaders(unicode); - using HttpClientHandler handler = CreateHttpClientHandler(); - var underlyingHandler = (SocketsHttpHandler)GetUnderlyingSocketsHttpHandler(handler); + LoopbackServerFactory.CreateClientAndServerAsync( + async uri => + { + using var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri) { Version = VersionFromUseHttp2 }; - using HttpClient client = CreateHttpClient(handler); + foreach ((string name, string[] values) in headers) + { + requestMessage.Headers.Add(name, values); + } + using HttpClient client = CreateHttpClient(); + + if (expectSuccess) + { await client.SendAsync(requestMessage); - }, - async server => + } + else { - HttpRequestData requestData = await server.HandleRequestAsync(); + await Assert.ThrowsAsync(() => client.SendAsync(requestMessage)); + } + }, + async server => + { + HttpRequestData requestData = null; + try + { + requestData = await server.HandleRequestAsync(); + } + catch (Exception) + { + if (expectSuccess) + throw; + } + + if (!expectSuccess) + return; + + Assert.All(requestData.Headers, + h => Assert.False(h.HuffmanEncoded, "Expose raw decoded bytes once HuffmanEncoding is supported")); - Assert.All(requestData.Headers, - h => Assert.False(h.HuffmanEncoded, "Expose raw decoded bytes once HuffmanEncoding is supported")); + Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); - Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); + foreach ((string name, string[] values) in headers) + { + byte[] valueBytes = valueEncoding.GetBytes(string.Join(", ", values)); - foreach ((string name, string[] values) in s_nonAsciiHeaders) - { - byte[] valueBytes = valueEncoding.GetBytes(string.Join(", ", values)); - Assert.Single(requestData.Headers, - h => h.Name.Equals(name, StringComparison.OrdinalIgnoreCase) && h.Raw.AsSpan().IndexOf(valueBytes) != -1); - } - }) + Assert.Single(requestData.Headers, + h => h.Name.Equals(name, StringComparison.OrdinalIgnoreCase) && h.Raw.AsSpan().IndexOf(valueBytes) != -1); + } + }) .GetAwaiter().GetResult(); - }, useAppCtxSwitchOuter.ToString()); + + return RemoteExecutor.SuccessExitCode; } - [Theory] - [InlineData(false)] - [InlineData(true)] - public void SendAsync_CustomResponseEncodingSelector_CanReceiveNonAsciiHeaderValues(bool useAppCtxSwitchOuter) + protected int SendAsync_ReceiveNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) { - RemoteExecutor.Invoke((useAppCtxSwitchInner) => - { - AllowNonAsciiHeaders(useAppCtxSwitchInner); + bool allowNonAscii = AllowNonAsciiHeaders(useAppCtxSwitchInner, switchValueInner); + bool unicode = bool.Parse(unicodeInner); + bool expectSuccess = allowNonAscii && !unicode; - LoopbackServerFactory.CreateClientAndServerAsync( + LoopbackServerFactory.CreateClientAndServerAsync( async uri => { - var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri); + using var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri) { Version = VersionFromUseHttp2 }; + using HttpClient client = CreateHttpClient(); - using HttpClientHandler handler = CreateHttpClientHandler(); - var underlyingHandler = (SocketsHttpHandler)GetUnderlyingSocketsHttpHandler(handler); + using HttpResponseMessage response = await client.SendAsync(requestMessage); - using HttpClient client = CreateHttpClient(handler); + if (!expectSuccess) + return; - using HttpResponseMessage response = await client.SendAsync(requestMessage); Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); - - foreach ((string name, string[] values) in s_nonAsciiHeaders) + foreach ((string name, string[] values) in GetNonAsciiTestHeaders(unicode)) { IEnumerable receivedValues = Assert.Single(response.Headers, h => h.Key.Equals(name, StringComparison.OrdinalIgnoreCase)).Value; string value = Assert.Single(receivedValues); @@ -385,14 +429,14 @@ public void SendAsync_CustomResponseEncodingSelector_CanReceiveNonAsciiHeaderVal }, async server => { - List headerData = s_nonAsciiHeaders - .Select(h => new HttpHeaderData(h.Name, string.Join(", ", h.Values))) + List headerData = GetNonAsciiTestHeaders(unicode) + .Select(h => new HttpHeaderData(h.Name, string.Join(", ", h.Values), latin1: true)) .ToList(); await server.HandleRequestAsync(headers: headerData); }) .GetAwaiter().GetResult(); - }, useAppCtxSwitchOuter.ToString()); + return RemoteExecutor.SuccessExitCode; } } } diff --git a/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index 86b306ebc341..0a4e5169df5c 100644 --- a/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -2255,6 +2255,35 @@ public SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http11(ITestOutputHe protected override bool UseSocketsHttpHandler => true; } + public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http11_NonAscii : HttpClientHandlerTest_Headers_NonAscii + { + [Theory] + [MemberData(nameof(NonAsciiTestConfigurations))] + public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) + { + //SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()); + + RemoteExecutor.Invoke( + (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitchInner, switchValueInner, utf8Inner), + useAppCtxSwitch.ToString(), + switchValue.ToString(), + unicode.ToString()) + .Dispose(); + } + + [Theory] + [MemberData(nameof(NonAsciiTestConfigurations))] + public void SendAsync_ReceiveNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) + { + RemoteExecutor.Invoke( + (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_ReceiveNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, utf8Inner), + useAppCtxSwitch.ToString(), + switchValue.ToString(), + unicode.ToString()) + .Dispose(); + } + } + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http2 : HttpClientHandlerTest_Headers { @@ -2263,6 +2292,39 @@ public SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http2(ITestOutputHel protected override bool UseHttp2 => true; } + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] + public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http2_NonAscii : HttpClientHandlerTest_Headers_NonAscii + { + protected override bool UseSocketsHttpHandler => true; + protected override bool UseHttp2 => true; + + [Theory] + [MemberData(nameof(NonAsciiTestConfigurations))] + public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) + { + //SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()); + + RemoteExecutor.Invoke( + (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitchInner, switchValueInner, utf8Inner), + useAppCtxSwitch.ToString(), + switchValue.ToString(), + unicode.ToString()) + .Dispose(); + } + + [Theory] + [MemberData(nameof(NonAsciiTestConfigurations))] + public void SendAsync_ReceiveNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) + { + RemoteExecutor.Invoke( + (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_ReceiveNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, utf8Inner), + useAppCtxSwitch.ToString(), + switchValue.ToString(), + unicode.ToString()) + .Dispose(); + } + } + [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.SupportsAlpn))] public sealed class SocketsHttpHandler_HttpClientHandler_Cancellation_Test_Http2 : HttpClientHandler_Cancellation_Test { From 9821bfea03b60ef7398b8ae6364a50a08f7219ef Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 17:13:59 +0200 Subject: [PATCH 05/15] validate for 0xFF when AllowNonAsciiHeaders is true --- .../SocketsHttpHandler/HPack/HPackEncoder.cs | 24 +++++------------ .../Http/SocketsHttpHandler/HttpConnection.cs | 27 +++++++------------ .../SocketsHttpHandler/StaticHttpSettings.cs | 2 ++ .../HttpClientHandlerTest.Headers.cs | 6 ++--- 4 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs index 7bf745df4e85..181f56a5854f 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs @@ -217,28 +217,18 @@ private static bool EncodeStringLiteralValue(string value, Span destinatio { if (value.Length <= destination.Length) { - if (StaticHttpSettings.AllowNonAsciiHeaders) + int mask = StaticHttpSettings.EncodingValidationMask; + for (int i = 0; i < value.Length; i++) { - for (int i = 0; i < value.Length; i++) + int c = value[i]; + if ((c & mask) != 0) { - char c = value[i]; - destination[i] = (byte)c; + throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); } - } - else - { - for (int i = 0; i < value.Length; i++) - { - char c = value[i]; - if ((c & 0xFF80) != 0) - { - throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); - } - destination[i] = (byte)c; - } + destination[i] = (byte)c; } - + bytesWritten = value.Length; return true; diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 43d821b9ee8e..c81b6a8cdc45 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -1147,25 +1147,16 @@ private Task WriteStringAsync(string s) if (s.Length <= _writeBuffer.Length - offset) { byte[] writeBuffer = _writeBuffer; - if (StaticHttpSettings.AllowNonAsciiHeaders) + int mask = StaticHttpSettings.EncodingValidationMask; + foreach (char c in s) { - foreach (char c in s) + if ((c & mask) != 0) { - writeBuffer[offset++] = (byte)c; - } - } - else - { - foreach (char c in s) - { - if ((c & 0xFF80) != 0) - { - throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); - } - writeBuffer[offset++] = (byte)c; + throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); } + writeBuffer[offset++] = (byte)c; } - + _writeOffset = offset; return Task.CompletedTask; } @@ -1197,11 +1188,13 @@ private Task WriteAsciiStringAsync(string s) private async Task WriteStringAsyncSlow(string s) { + int mask = StaticHttpSettings.EncodingValidationMask; + for (int i = 0; i < s.Length; i++) { - char c = s[i]; + int c = s[i]; - if (!StaticHttpSettings.AllowNonAsciiHeaders && (c & 0xFF80) != 0) + if ((c & mask) != 0) { throw new HttpRequestException(SR.net_http_request_invalid_char_encoding); } diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs index 35633249c2b3..d32a640bd8ef 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs @@ -18,6 +18,8 @@ internal static class StaticHttpSettings // before the first calls to HttpClient API-s. internal static bool AllowNonAsciiHeaders => s_allowNonAsciiHeaders.Value; + internal static int EncodingValidationMask => AllowNonAsciiHeaders ? 0xFF00 : 0xFF80; + private static bool GetAllowNonAsciiCharactersSetting() { // First check for the AppContext switch, giving it priority over the environment variable. diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index 08053facdd8a..d4df89f7664c 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -298,10 +298,8 @@ public HttpClientHandlerTest_Headers_NonAscii() : base(null) private static readonly (string Name, string[] Values)[] s_validLatin1Characters = new[] { - ("a", new[] { "aa"}) - //("a", new[] { "\x0081"}) - //("Header1", new[] { "\x0081\x00FF\x00FE", "ascii-only" }), - //("Set-Cookie", new[] { "\u00FF", "\x00B0\x00B1\x00B2", "Ascii\x00B8" }), + ("Header1", new[] { "\x0081\x00FF\x00FE", "ascii-only" }), + ("Set-Cookie", new[] { "\u00FF", "\x00B0\x00B1\x00B2", "Ascii\x00B8" }), }; private static readonly (string Name, string[] Values)[] s_invalidUnicodeCharacters = new[] From bb2424f9e0261ccccb2396795cb7f05c496de4aa Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 17:31:45 +0200 Subject: [PATCH 06/15] MultipartContentTest.ReadAsStreamAsync_CanEncodeLatin1 --- .../FunctionalTests/MultipartContentTest.cs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs b/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs index c3b670cc9835..9f6bd172d116 100644 --- a/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.IO; +using System.Linq; using System.Text; using System.Threading.Tasks; @@ -361,6 +362,44 @@ public async Task ReadAsStreamAsync_CreateContentReadStreamAsyncThrows_Exception await Assert.ThrowsAsync(() => t); } + [Fact] + public async Task ReadAsStreamAsync_CanEncodeLatin1() + { + var mc = new MultipartContent("subtype", "fooBoundary"); + + var stringContent = new StringContent("bar1"); + stringContent.Headers.Add("latin1", "\uD83D\uDE00"); + mc.Add(stringContent); + + var byteArrayContent = new ByteArrayContent(Encoding.ASCII.GetBytes("bar4")); + byteArrayContent.Headers.Add("default", "\uD83D\uDE00"); + mc.Add(byteArrayContent); + + var ms = new MemoryStream(); + await (await mc.ReadAsStreamAsync()).CopyToAsync(ms); + + Encoding latin1 = Encoding.GetEncoding("ISO-8859-1"); + + byte[] expected = Concat( + latin1.GetBytes("--fooBoundary\r\n"), + latin1.GetBytes("Content-Type: text/plain; charset=utf-8\r\n"), + latin1.GetBytes("latin1: "), + latin1.GetBytes("\uD83D\uDE00"), + latin1.GetBytes("\r\n\r\n"), + latin1.GetBytes("bar1"), + latin1.GetBytes("\r\n--fooBoundary\r\n"), + latin1.GetBytes("default: "), + latin1.GetBytes("\uD83D\uDE00"), + latin1.GetBytes("\r\n\r\n"), + latin1.GetBytes("bar4"), + latin1.GetBytes("\r\n--fooBoundary--\r\n")); + + byte[] actual = ms.ToArray(); + Assert.Equal(expected, actual); + + static byte[] Concat(params byte[][] arrays) => arrays.SelectMany(b => b).ToArray(); + } + #region Helpers private static async Task MultipartContentToStringAsync(MultipartContent content, MultipartContentToStringMode mode) From dd9efa5f9b04a9e81a07f97afd56da0197d6a8f6 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 17:34:42 +0200 Subject: [PATCH 07/15] remove commented lines --- .../tests/FunctionalTests/SocketsHttpHandlerTest.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index 0a4e5169df5c..c0abad7a9dba 100644 --- a/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -2261,8 +2261,6 @@ public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http11_ [MemberData(nameof(NonAsciiTestConfigurations))] public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) { - //SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()); - RemoteExecutor.Invoke( (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitchInner, switchValueInner, utf8Inner), useAppCtxSwitch.ToString(), @@ -2302,8 +2300,6 @@ public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http2_N [MemberData(nameof(NonAsciiTestConfigurations))] public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) { - //SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()); - RemoteExecutor.Invoke( (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitchInner, switchValueInner, utf8Inner), useAppCtxSwitch.ToString(), From 43f534b85042b1f0091d4b8b2dbac96f6d3761be Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 19:07:53 +0200 Subject: [PATCH 08/15] rename switches --- .../Http/SocketsHttpHandler/StaticHttpSettings.cs | 12 ++++++------ .../FunctionalTests/HttpClientHandlerTest.Headers.cs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs index d32a640bd8ef..15e78d132a2c 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs @@ -6,21 +6,21 @@ namespace System.Net.Http { internal static class StaticHttpSettings { - private const string AllowNonAsciiCharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS"; - private const string AllowNonAsciiCharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders"; + private const string AllowNonAsciiCharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWLATIN1HEADERS"; + private const string AllowNonAsciiCharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowLatin1Headers"; - private static readonly Lazy s_allowNonAsciiHeaders = new Lazy(GetAllowNonAsciiCharactersSetting); + private static readonly Lazy s_allowLatin1Headers = new Lazy(GetAllowLatin1HeadersSetting); // Disables a validation that checks whether Http headers contain a non-ASCII character. // This is a workaround that has been introduced as a patch specific to the 3.1 branch. // Unlike options in HttpConnectionSettings, this one has a global scope. // Lazy initialization is being used to make sure clients can also use AppContext.SetSwitch() or Environment.SetEnvironmentVariable() // before the first calls to HttpClient API-s. - internal static bool AllowNonAsciiHeaders => s_allowNonAsciiHeaders.Value; + internal static bool AllowLatin1Headers => s_allowLatin1Headers.Value; - internal static int EncodingValidationMask => AllowNonAsciiHeaders ? 0xFF00 : 0xFF80; + internal static int EncodingValidationMask => AllowLatin1Headers ? 0xFF00 : 0xFF80; - private static bool GetAllowNonAsciiCharactersSetting() + private static bool GetAllowLatin1HeadersSetting() { // First check for the AppContext switch, giving it priority over the environment variable. if (AppContext.TryGetSwitch(AllowNonAsciiCharactersAppCtxSettingName, out bool value)) diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index d4df89f7664c..f3595d2d3a73 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -315,11 +315,11 @@ private static bool AllowNonAsciiHeaders(string useAppCtxSwitchInner, string swi bool switchValue = bool.Parse(switchValueInner); if (bool.Parse(useAppCtxSwitchInner)) { - AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.AllowNonAsciiHeaders", switchValue); + AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.AllowLatin1Headers", switchValue); } else { - Environment.SetEnvironmentVariable("DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWNONASCIIHEADERS", switchValueInner); + Environment.SetEnvironmentVariable("DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWLATIN1HEADERS", switchValueInner); } return switchValue; } From db83595b923cb9e222b3ca1991bf5ca7a7dac218 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 19:50:13 +0200 Subject: [PATCH 09/15] fix Http2 tests --- .../tests/System/Net/Http/Http2LoopbackConnection.cs | 12 +++++++----- src/Common/tests/System/Net/Http/LoopbackServer.cs | 3 ++- .../FunctionalTests/HttpClientHandlerTest.Headers.cs | 12 ++++++------ 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index 94c260d18714..4ee682d2a8de 100644 --- a/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -17,6 +17,8 @@ namespace System.Net.Test.Common { public class Http2LoopbackConnection : GenericLoopbackConnection { + private static readonly Encoding s_latin1Encoding = Encoding.GetEncoding("ISO-8859-1"); + private Socket _connectionSocket; private Stream _connectionStream; private TaskCompletionSource _ignoredSettingsAckPromise; @@ -381,9 +383,9 @@ private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan } } - private static int EncodeString(string value, Span headerBlock, bool huffmanEncode) + private static int EncodeString(string value, Span headerBlock, bool huffmanEncode, bool latin1 = false) { - byte[] data = Encoding.ASCII.GetBytes(value); + byte[] data = (latin1 ? s_latin1Encoding : Encoding.ASCII).GetBytes(value); byte prefix; if (!huffmanEncode) @@ -536,12 +538,12 @@ private static (int bytesConsumed, HttpHeaderData headerData) DecodeHeader(ReadO } } - public static int EncodeHeader(HttpHeaderData headerData, Span headerBlock) + public static int EncodeHeader(HttpHeaderData headerData, Span headerBlock, bool latin1 = false) { // Always encode as literal, no indexing. int bytesGenerated = EncodeInteger(0, 0, 0b11110000, headerBlock); bytesGenerated += EncodeString(headerData.Name, headerBlock.Slice(bytesGenerated), headerData.HuffmanEncoded); - bytesGenerated += EncodeString(headerData.Value, headerBlock.Slice(bytesGenerated), headerData.HuffmanEncoded); + bytesGenerated += EncodeString(headerData.Value, headerBlock.Slice(bytesGenerated), headerData.HuffmanEncoded, latin1); return bytesGenerated; } @@ -680,7 +682,7 @@ public async Task SendResponseHeadersAsync(int streamId, bool endStream = true, { foreach (HttpHeaderData headerData in headers) { - bytesGenerated += EncodeHeader(headerData, headerBlock.AsSpan().Slice(bytesGenerated)); + bytesGenerated += EncodeHeader(headerData, headerBlock.AsSpan().Slice(bytesGenerated), headerData.Latin1); } } diff --git a/src/Common/tests/System/Net/Http/LoopbackServer.cs b/src/Common/tests/System/Net/Http/LoopbackServer.cs index 129aeb3fb24e..88a501eb7eab 100644 --- a/src/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/Common/tests/System/Net/Http/LoopbackServer.cs @@ -18,6 +18,7 @@ public sealed partial class LoopbackServer : GenericLoopbackServer, IDisposable { private static readonly byte[] s_newLineBytes = new byte[] { (byte)'\r', (byte)'\n' }; private static readonly byte[] s_colonSpaceBytes = new byte[] { (byte)':', (byte)' ' }; + private static readonly Encoding s_latin1Encoding = Encoding.GetEncoding("ISO-8859-1"); private Socket _listenSocket; private Options _options; @@ -805,7 +806,7 @@ public override async Task SendResponseAsync(HttpStatusCode? statusCode = HttpSt headerBytes.Write(nameBytes, 0, nameBytes.Length); headerBytes.Write(s_colonSpaceBytes, 0, s_colonSpaceBytes.Length); - byte[] valueBytes = (headerData.Latin1 ? Encoding.GetEncoding("ISO-8859-1") : Encoding.ASCII).GetBytes(headerData.Value); + byte[] valueBytes = (headerData.Latin1 ? s_latin1Encoding : Encoding.ASCII).GetBytes(headerData.Value); headerBytes.Write(valueBytes, 0, valueBytes.Length); headerBytes.Write(s_newLineBytes, 0, s_newLineBytes.Length); } diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index f3595d2d3a73..cd9f07563141 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -310,7 +310,7 @@ private static readonly (string Name, string[] Values)[] s_invalidUnicodeCharact private static (string Name, string[] Values)[] GetNonAsciiTestHeaders(bool unicode) => unicode ? s_invalidUnicodeCharacters : s_validLatin1Characters; - private static bool AllowNonAsciiHeaders(string useAppCtxSwitchInner, string switchValueInner) + private static bool AllowLatin1Headers(string useAppCtxSwitchInner, string switchValueInner) { bool switchValue = bool.Parse(switchValueInner); if (bool.Parse(useAppCtxSwitchInner)) @@ -337,9 +337,9 @@ private static bool AllowNonAsciiHeaders(string useAppCtxSwitchInner, string swi // Since RemoteExecutor cannot invoke delegates defined in abstract types, we should define the actual test cases in derived classes. protected int SendAsync_SendNonAsciiCharacters_Impl(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) { - bool allowNonAscii = AllowNonAsciiHeaders(useAppCtxSwitchInner, switchValueInner); + bool allowLatin1 = AllowLatin1Headers(useAppCtxSwitchInner, switchValueInner); bool unicode = bool.Parse(unicodeInner); - bool expectSuccess = allowNonAscii && !unicode; + bool expectSuccess = allowLatin1 && !unicode; (string name, string[] values)[] headers = GetNonAsciiTestHeaders(unicode); @@ -400,9 +400,9 @@ protected int SendAsync_SendNonAsciiCharacters_Impl(string useAppCtxSwitchInner, protected int SendAsync_ReceiveNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) { - bool allowNonAscii = AllowNonAsciiHeaders(useAppCtxSwitchInner, switchValueInner); + bool allowLatin1 = AllowLatin1Headers(useAppCtxSwitchInner, switchValueInner); bool unicode = bool.Parse(unicodeInner); - bool expectSuccess = allowNonAscii && !unicode; + bool expectSuccess = allowLatin1 && !unicode; LoopbackServerFactory.CreateClientAndServerAsync( async uri => @@ -428,7 +428,7 @@ protected int SendAsync_ReceiveNonAsciiCharacters_Inner(string useAppCtxSwitchIn async server => { List headerData = GetNonAsciiTestHeaders(unicode) - .Select(h => new HttpHeaderData(h.Name, string.Join(", ", h.Values), latin1: true)) + .Select(h => new HttpHeaderData(h.Name, string.Join(", ", h.Values), latin1: allowLatin1)) .ToList(); await server.HandleRequestAsync(headers: headerData); From c4793d0ac149cc2784133fde1755c91828db829b Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 19:57:17 +0200 Subject: [PATCH 10/15] naming --- .../FunctionalTests/HttpClientHandlerTest.Headers.cs | 2 +- .../tests/FunctionalTests/SocketsHttpHandlerTest.cs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index cd9f07563141..5c1230d7a358 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -335,7 +335,7 @@ private static bool AllowLatin1Headers(string useAppCtxSwitchInner, string switc }; // Since RemoteExecutor cannot invoke delegates defined in abstract types, we should define the actual test cases in derived classes. - protected int SendAsync_SendNonAsciiCharacters_Impl(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) + protected int SendAsync_SendNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) { bool allowLatin1 = AllowLatin1Headers(useAppCtxSwitchInner, switchValueInner); bool unicode = bool.Parse(unicodeInner); diff --git a/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs index c0abad7a9dba..497c4945f16d 100644 --- a/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs @@ -2262,7 +2262,7 @@ public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http11_ public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) { RemoteExecutor.Invoke( - (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitchInner, switchValueInner, utf8Inner), + (useAppCtxSwitchInner, switchValueInner, unicodeInner) => SendAsync_SendNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, unicodeInner), useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()) @@ -2274,7 +2274,7 @@ public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchVa public void SendAsync_ReceiveNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) { RemoteExecutor.Invoke( - (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_ReceiveNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, utf8Inner), + (useAppCtxSwitchInner, switchValueInner, unicodeInner) => SendAsync_ReceiveNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, unicodeInner), useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()) @@ -2301,7 +2301,7 @@ public sealed class SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http2_N public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) { RemoteExecutor.Invoke( - (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_SendNonAsciiCharacters_Impl(useAppCtxSwitchInner, switchValueInner, utf8Inner), + (useAppCtxSwitchInner, switchValueInner, unicodeInner) => SendAsync_SendNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, unicodeInner), useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()) @@ -2313,7 +2313,7 @@ public void SendAsync_SendNonAsciiCharacters(bool useAppCtxSwitch, bool switchVa public void SendAsync_ReceiveNonAsciiCharacters(bool useAppCtxSwitch, bool switchValue, bool unicode) { RemoteExecutor.Invoke( - (useAppCtxSwitchInner, switchValueInner, utf8Inner) => SendAsync_ReceiveNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, utf8Inner), + (useAppCtxSwitchInner, switchValueInner, unicodeInner) => SendAsync_ReceiveNonAsciiCharacters_Inner(useAppCtxSwitchInner, switchValueInner, unicodeInner), useAppCtxSwitch.ToString(), switchValue.ToString(), unicode.ToString()) From 84ae85aa2b47cc586e703f0611b9dde59242ade7 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 22:22:28 +0200 Subject: [PATCH 11/15] try fixing CI builds --- src/System.Net.Http/src/System.Net.Http.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Net.Http/src/System.Net.Http.csproj b/src/System.Net.Http/src/System.Net.Http.csproj index 8bdb3fb3e875..01e8f2a746b9 100644 --- a/src/System.Net.Http/src/System.Net.Http.csproj +++ b/src/System.Net.Http/src/System.Net.Http.csproj @@ -93,6 +93,7 @@ + Common\CoreLib\System\IO\StreamHelpers.CopyValidation.cs @@ -161,7 +162,6 @@ - From fee785dde8df84611846a11390f416993bde9f40 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 8 Sep 2020 22:36:06 +0200 Subject: [PATCH 12/15] address review findings --- .../SocketsHttpHandler/HPack/HPackEncoder.cs | 1 - .../SocketsHttpHandler/StaticHttpSettings.cs | 8 +++--- .../HttpClientHandlerTest.Headers.cs | 28 +++++++++---------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs index 181f56a5854f..424e1fff043d 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HPack/HPackEncoder.cs @@ -229,7 +229,6 @@ private static bool EncodeStringLiteralValue(string value, Span destinatio destination[i] = (byte)c; } - bytesWritten = value.Length; return true; } diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs index 15e78d132a2c..7fbca275f40c 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs @@ -6,8 +6,8 @@ namespace System.Net.Http { internal static class StaticHttpSettings { - private const string AllowNonAsciiCharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWLATIN1HEADERS"; - private const string AllowNonAsciiCharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowLatin1Headers"; + private const string AllowLatin1CharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWLATIN1HEADERS"; + private const string AllowLatin1CharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowLatin1Headers"; private static readonly Lazy s_allowLatin1Headers = new Lazy(GetAllowLatin1HeadersSetting); @@ -23,13 +23,13 @@ internal static class StaticHttpSettings private static bool GetAllowLatin1HeadersSetting() { // First check for the AppContext switch, giving it priority over the environment variable. - if (AppContext.TryGetSwitch(AllowNonAsciiCharactersAppCtxSettingName, out bool value)) + if (AppContext.TryGetSwitch(AllowLatin1CharactersAppCtxSettingName, out bool value)) { return value; } // AppContext switch wasn't used. Check the environment variable. - string envVar = Environment.GetEnvironmentVariable(AllowNonAsciiCharactersEnvironmentVariableSettingName); + string envVar = Environment.GetEnvironmentVariable(AllowLatin1CharactersEnvironmentVariableSettingName); if (envVar != null && (envVar.Equals("true", StringComparison.OrdinalIgnoreCase) || envVar.Equals("1"))) { return true; diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index 5c1230d7a358..646638667e55 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -161,10 +161,10 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri => Assert.Null(response.Content.Headers.Expires); } }, - async server => - { - await server.HandleRequestAsync(HttpStatusCode.OK); - }); + async server => + { + await server.HandleRequestAsync(HttpStatusCode.OK); + }); } [Theory] @@ -335,7 +335,7 @@ private static bool AllowLatin1Headers(string useAppCtxSwitchInner, string switc }; // Since RemoteExecutor cannot invoke delegates defined in abstract types, we should define the actual test cases in derived classes. - protected int SendAsync_SendNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) + protected async Task SendAsync_SendNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) { bool allowLatin1 = AllowLatin1Headers(useAppCtxSwitchInner, switchValueInner); bool unicode = bool.Parse(unicodeInner); @@ -343,7 +343,7 @@ protected int SendAsync_SendNonAsciiCharacters_Inner(string useAppCtxSwitchInner (string name, string[] values)[] headers = GetNonAsciiTestHeaders(unicode); - LoopbackServerFactory.CreateClientAndServerAsync( + await LoopbackServerFactory.CreateClientAndServerAsync( async uri => { using var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri) { Version = VersionFromUseHttp2 }; @@ -371,10 +371,9 @@ protected int SendAsync_SendNonAsciiCharacters_Inner(string useAppCtxSwitchInner { requestData = await server.HandleRequestAsync(); } - catch (Exception) + catch (Exception) when (!expectSuccess) { - if (expectSuccess) - throw; + // Eat up expected exceptions } if (!expectSuccess) @@ -392,19 +391,18 @@ protected int SendAsync_SendNonAsciiCharacters_Inner(string useAppCtxSwitchInner Assert.Single(requestData.Headers, h => h.Name.Equals(name, StringComparison.OrdinalIgnoreCase) && h.Raw.AsSpan().IndexOf(valueBytes) != -1); } - }) - .GetAwaiter().GetResult(); + }); return RemoteExecutor.SuccessExitCode; } - protected int SendAsync_ReceiveNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) + protected async Task SendAsync_ReceiveNonAsciiCharacters_Inner(string useAppCtxSwitchInner, string switchValueInner, string unicodeInner) { bool allowLatin1 = AllowLatin1Headers(useAppCtxSwitchInner, switchValueInner); bool unicode = bool.Parse(unicodeInner); bool expectSuccess = allowLatin1 && !unicode; - LoopbackServerFactory.CreateClientAndServerAsync( + await LoopbackServerFactory.CreateClientAndServerAsync( async uri => { using var requestMessage = new HttpRequestMessage(HttpMethod.Get, uri) { Version = VersionFromUseHttp2 }; @@ -432,8 +430,8 @@ protected int SendAsync_ReceiveNonAsciiCharacters_Inner(string useAppCtxSwitchIn .ToList(); await server.HandleRequestAsync(headers: headerData); - }) - .GetAwaiter().GetResult(); + }); + return RemoteExecutor.SuccessExitCode; } } From aae055d1486a509f36fa22cba9955b12714b7bb2 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 9 Sep 2020 17:00:52 +0200 Subject: [PATCH 13/15] remove Lazy --- .../System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs index 7fbca275f40c..3fc314dfa19f 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/StaticHttpSettings.cs @@ -9,14 +9,12 @@ internal static class StaticHttpSettings private const string AllowLatin1CharactersEnvironmentVariableSettingName = "DOTNET_SYSTEM_NET_HTTP_SOCKETSHTTPHANDLER_ALLOWLATIN1HEADERS"; private const string AllowLatin1CharactersAppCtxSettingName = "System.Net.Http.SocketsHttpHandler.AllowLatin1Headers"; - private static readonly Lazy s_allowLatin1Headers = new Lazy(GetAllowLatin1HeadersSetting); - // Disables a validation that checks whether Http headers contain a non-ASCII character. // This is a workaround that has been introduced as a patch specific to the 3.1 branch. // Unlike options in HttpConnectionSettings, this one has a global scope. // Lazy initialization is being used to make sure clients can also use AppContext.SetSwitch() or Environment.SetEnvironmentVariable() // before the first calls to HttpClient API-s. - internal static bool AllowLatin1Headers => s_allowLatin1Headers.Value; + internal static bool AllowLatin1Headers { get; } = GetAllowLatin1HeadersSetting(); internal static int EncodingValidationMask => AllowLatin1Headers ? 0xFF00 : 0xFF80; From 48bccd08e42ad2f23be19651024f1e64bc9c18ff Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Wed, 9 Sep 2020 17:09:27 +0200 Subject: [PATCH 14/15] add shared Latin1Encoding field to HttpHeaderData --- src/Common/tests/System/Net/Http/GenericLoopbackServer.cs | 2 ++ src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs | 4 +--- src/Common/tests/System/Net/Http/LoopbackServer.cs | 3 +-- .../tests/FunctionalTests/HttpClientHandlerTest.Headers.cs | 4 ++-- .../tests/FunctionalTests/MultipartContentTest.cs | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs b/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs index 9110eefcbbab..4c85fd7542ad 100644 --- a/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs +++ b/src/Common/tests/System/Net/Http/GenericLoopbackServer.cs @@ -83,6 +83,8 @@ public class GenericLoopbackOptions public struct HttpHeaderData { + public static readonly Encoding Latin1Encoding = Encoding.GetEncoding("ISO-8859-1"); + public string Name { get; } public string Value { get; } public bool HuffmanEncoded { get; } diff --git a/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs b/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs index 4ee682d2a8de..5bc66cc01a1f 100644 --- a/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs +++ b/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs @@ -17,8 +17,6 @@ namespace System.Net.Test.Common { public class Http2LoopbackConnection : GenericLoopbackConnection { - private static readonly Encoding s_latin1Encoding = Encoding.GetEncoding("ISO-8859-1"); - private Socket _connectionSocket; private Stream _connectionStream; private TaskCompletionSource _ignoredSettingsAckPromise; @@ -385,7 +383,7 @@ private static (int bytesConsumed, string value) DecodeString(ReadOnlySpan private static int EncodeString(string value, Span headerBlock, bool huffmanEncode, bool latin1 = false) { - byte[] data = (latin1 ? s_latin1Encoding : Encoding.ASCII).GetBytes(value); + byte[] data = (latin1 ? HttpHeaderData.Latin1Encoding : Encoding.ASCII).GetBytes(value); byte prefix; if (!huffmanEncode) diff --git a/src/Common/tests/System/Net/Http/LoopbackServer.cs b/src/Common/tests/System/Net/Http/LoopbackServer.cs index 88a501eb7eab..355a6384f625 100644 --- a/src/Common/tests/System/Net/Http/LoopbackServer.cs +++ b/src/Common/tests/System/Net/Http/LoopbackServer.cs @@ -18,7 +18,6 @@ public sealed partial class LoopbackServer : GenericLoopbackServer, IDisposable { private static readonly byte[] s_newLineBytes = new byte[] { (byte)'\r', (byte)'\n' }; private static readonly byte[] s_colonSpaceBytes = new byte[] { (byte)':', (byte)' ' }; - private static readonly Encoding s_latin1Encoding = Encoding.GetEncoding("ISO-8859-1"); private Socket _listenSocket; private Options _options; @@ -806,7 +805,7 @@ public override async Task SendResponseAsync(HttpStatusCode? statusCode = HttpSt headerBytes.Write(nameBytes, 0, nameBytes.Length); headerBytes.Write(s_colonSpaceBytes, 0, s_colonSpaceBytes.Length); - byte[] valueBytes = (headerData.Latin1 ? s_latin1Encoding : Encoding.ASCII).GetBytes(headerData.Value); + byte[] valueBytes = (headerData.Latin1 ? HttpHeaderData.Latin1Encoding : Encoding.ASCII).GetBytes(headerData.Value); headerBytes.Write(valueBytes, 0, valueBytes.Length); headerBytes.Write(s_newLineBytes, 0, s_newLineBytes.Length); } diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs index 646638667e55..0532e6697298 100644 --- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs +++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs @@ -382,7 +382,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync( Assert.All(requestData.Headers, h => Assert.False(h.HuffmanEncoded, "Expose raw decoded bytes once HuffmanEncoding is supported")); - Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); + Encoding valueEncoding = HttpHeaderData.Latin1Encoding; foreach ((string name, string[] values) in headers) { @@ -413,7 +413,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync( if (!expectSuccess) return; - Encoding valueEncoding = Encoding.GetEncoding("ISO-8859-1"); + Encoding valueEncoding = HttpHeaderData.Latin1Encoding; foreach ((string name, string[] values) in GetNonAsciiTestHeaders(unicode)) { IEnumerable receivedValues = Assert.Single(response.Headers, h => h.Key.Equals(name, StringComparison.OrdinalIgnoreCase)).Value; diff --git a/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs b/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs index 9f6bd172d116..846e6f6ebfc3 100644 --- a/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs +++ b/src/System.Net.Http/tests/FunctionalTests/MultipartContentTest.cs @@ -378,7 +378,7 @@ public async Task ReadAsStreamAsync_CanEncodeLatin1() var ms = new MemoryStream(); await (await mc.ReadAsStreamAsync()).CopyToAsync(ms); - Encoding latin1 = Encoding.GetEncoding("ISO-8859-1"); + Encoding latin1 = Test.Common.HttpHeaderData.Latin1Encoding; byte[] expected = Concat( latin1.GetBytes("--fooBoundary\r\n"), From e8397af48d5abd701261e13859d86ad73004704a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Fri, 11 Sep 2020 12:04:36 +0200 Subject: [PATCH 15/15] changed foreach loop for consistency --- .../src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index c81b6a8cdc45..fe87ca09567e 100644 --- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -1148,7 +1148,7 @@ private Task WriteStringAsync(string s) { byte[] writeBuffer = _writeBuffer; int mask = StaticHttpSettings.EncodingValidationMask; - foreach (char c in s) + foreach (int c in s) { if ((c & mask) != 0) {