diff --git a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackEncoder.cs b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackEncoder.cs index 5c0bbecfcc7dc..dab588146dc2a 100644 --- a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackEncoder.cs +++ b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackEncoder.cs @@ -285,12 +285,7 @@ private static bool EncodeLiteralHeaderNewNameCore(byte mask, string name, strin } /// Encodes a "Literal Header Field without Indexing - New Name". - public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan values, string separator, Span destination, out int bytesWritten) - { - return EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, separator, valueEncoding: null, destination, out bytesWritten); - } - - public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan values, string separator, Encoding? valueEncoding, Span destination, out int bytesWritten) + public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan values, byte[] separator, Encoding? valueEncoding, Span destination, out int bytesWritten) { // From https://tools.ietf.org/html/rfc7541#section-6.2.2 // ------------------------------------------------------ @@ -515,12 +510,7 @@ public static bool EncodeDynamicTableSizeUpdate(int value, Span destinatio return false; } - public static bool EncodeStringLiterals(ReadOnlySpan values, string? separator, Span destination, out int bytesWritten) - { - return EncodeStringLiterals(values, separator, valueEncoding: null, destination, out bytesWritten); - } - - public static bool EncodeStringLiterals(ReadOnlySpan values, string? separator, Encoding? valueEncoding, Span destination, out int bytesWritten) + public static bool EncodeStringLiterals(ReadOnlySpan values, byte[]? separator, Encoding? valueEncoding, Span destination, out int bytesWritten) { bytesWritten = 0; @@ -536,23 +526,22 @@ public static bool EncodeStringLiterals(ReadOnlySpan values, string? sep if (destination.Length != 0) { Debug.Assert(separator != null); - int valueLength; + Debug.Assert(Ascii.IsValid(separator)); + int valueLength = checked((values.Length - 1) * separator.Length); - // Calculate length of all parts and separators. + // Calculate length of all values. if (valueEncoding is null || ReferenceEquals(valueEncoding, Encoding.Latin1)) { - valueLength = checked((int)(values.Length - 1) * separator.Length); foreach (string part in values) { - valueLength = checked((int)(valueLength + part.Length)); + valueLength = checked(valueLength + part.Length); } } else { - valueLength = checked((int)(values.Length - 1) * valueEncoding.GetByteCount(separator)); foreach (string part in values) { - valueLength = checked((int)(valueLength + valueEncoding.GetByteCount(part))); + valueLength = checked(valueLength + valueEncoding.GetByteCount(part)); } } @@ -571,7 +560,7 @@ public static bool EncodeStringLiterals(ReadOnlySpan values, string? sep for (int i = 1; i < values.Length; i++) { - EncodeValueStringPart(separator, destination); + separator.CopyTo(destination); destination = destination.Slice(separator.Length); value = values[i]; @@ -586,8 +575,8 @@ public static bool EncodeStringLiterals(ReadOnlySpan values, string? sep for (int i = 1; i < values.Length; i++) { - written = valueEncoding.GetBytes(separator, destination); - destination = destination.Slice(written); + separator.CopyTo(destination); + destination = destination.Slice(separator.Length); written = valueEncoding.GetBytes(values[i], destination); destination = destination.Slice(written); diff --git a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackEncoder.cs b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackEncoder.cs index aa951b2497199..5d96530b457d0 100644 --- a/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackEncoder.cs +++ b/src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackEncoder.cs @@ -144,14 +144,9 @@ public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, str /// /// Encodes a Literal Header Field Without Name Reference, building the value by concatenating a collection of strings with separators. /// - public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, ReadOnlySpan values, string valueSeparator, Span destination, out int bytesWritten) + public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, ReadOnlySpan values, byte[] separator, Encoding? valueEncoding, Span destination, out int bytesWritten) { - return EncodeLiteralHeaderFieldWithoutNameReference(name, values, valueSeparator, valueEncoding: null, destination, out bytesWritten); - } - - public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, ReadOnlySpan values, string valueSeparator, Encoding? valueEncoding, Span destination, out int bytesWritten) - { - if (EncodeNameString(name, destination, out int nameLength) && EncodeValueString(values, valueSeparator, valueEncoding, destination.Slice(nameLength), out int valueLength)) + if (EncodeNameString(name, destination, out int nameLength) && EncodeValueString(values, separator, valueEncoding, destination.Slice(nameLength), out int valueLength)) { bytesWritten = nameLength + valueLength; return true; @@ -222,12 +217,7 @@ private static bool EncodeValueString(string s, Encoding? valueEncoding, Span /// Encodes a value by concatenating a collection of strings, separated by a separator string. /// - public static bool EncodeValueString(ReadOnlySpan values, string? separator, Span buffer, out int length) - { - return EncodeValueString(values, separator, valueEncoding: null, buffer, out length); - } - - public static bool EncodeValueString(ReadOnlySpan values, string? separator, Encoding? valueEncoding, Span buffer, out int length) + public static bool EncodeValueString(ReadOnlySpan values, byte[]? separator, Encoding? valueEncoding, Span buffer, out int length) { if (values.Length == 1) { @@ -243,10 +233,11 @@ public static bool EncodeValueString(ReadOnlySpan values, string? separa if (buffer.Length > 0) { Debug.Assert(separator != null); - int valueLength; + Debug.Assert(Ascii.IsValid(separator)); + int valueLength = separator.Length * (values.Length - 1); + if (valueEncoding is null || ReferenceEquals(valueEncoding, Encoding.Latin1)) { - valueLength = separator.Length * (values.Length - 1); foreach (string part in values) { valueLength += part.Length; @@ -254,7 +245,6 @@ public static bool EncodeValueString(ReadOnlySpan values, string? separa } else { - valueLength = valueEncoding.GetByteCount(separator) * (values.Length - 1); foreach (string part in values) { valueLength += valueEncoding.GetByteCount(part); @@ -275,7 +265,7 @@ public static bool EncodeValueString(ReadOnlySpan values, string? separa for (int i = 1; i < values.Length; i++) { - EncodeValueStringPart(separator, buffer); + separator.CopyTo(buffer); buffer = buffer.Slice(separator.Length); value = values[i]; @@ -290,8 +280,8 @@ public static bool EncodeValueString(ReadOnlySpan values, string? separa for (int i = 1; i < values.Length; i++) { - written = valueEncoding.GetBytes(separator, buffer); - buffer = buffer.Slice(written); + separator.CopyTo(buffer); + buffer = buffer.Slice(separator.Length); written = valueEncoding.GetBytes(values[i], buffer); buffer = buffer.Slice(written); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs index fac5d58bf282d..a4d44b2a30714 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs @@ -277,5 +277,9 @@ private static bool TryDecodeUtf8(ReadOnlySpan input, [NotNullWhen(true)] decoded = null; return false; } + + public string Separator => Parser is { } parser ? parser.Separator : HttpHeaderParser.DefaultSeparator; + + public byte[] SeparatorBytes => Parser is { } parser ? parser.SeparatorBytes : HttpHeaderParser.DefaultSeparatorBytes; } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderStringValues.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderStringValues.cs index a313a2306e78f..6b5f4d2a666a2 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderStringValues.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderStringValues.cs @@ -45,7 +45,7 @@ internal HeaderStringValues(HeaderDescriptor descriptor, string[] values) public override string ToString() => _value switch { string value => value, - string[] values => string.Join(_header.Parser is HttpHeaderParser parser && parser.SupportsMultipleValues ? parser.Separator : HttpHeaderParser.DefaultSeparator, values), + string[] values => string.Join(_header.Separator, values), _ => string.Empty, }; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderParser.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderParser.cs index 2fa79f1f41812..711e37cf14694 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderParser.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderParser.cs @@ -4,53 +4,42 @@ using System.Collections; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Text; namespace System.Net.Http.Headers { internal abstract class HttpHeaderParser { - internal const string DefaultSeparator = ", "; + public const string DefaultSeparator = ", "; + public static readonly byte[] DefaultSeparatorBytes = ", "u8.ToArray(); - private readonly bool _supportsMultipleValues; - private readonly string? _separator; + public bool SupportsMultipleValues { get; private set; } - public bool SupportsMultipleValues - { - get { return _supportsMultipleValues; } - } + public string Separator { get; private set; } - public string? Separator - { - get - { - Debug.Assert(_supportsMultipleValues); - return _separator; - } - } + public byte[] SeparatorBytes { get; private set; } // If ValueType implements Equals() as required, there is no need to provide a comparer. A comparer is needed // e.g. if we want to compare strings using case-insensitive comparison. - public virtual IEqualityComparer? Comparer - { - get { return null; } - } + public virtual IEqualityComparer? Comparer => null; protected HttpHeaderParser(bool supportsMultipleValues) { - _supportsMultipleValues = supportsMultipleValues; - - if (supportsMultipleValues) - { - _separator = DefaultSeparator; - } + SupportsMultipleValues = supportsMultipleValues; + Separator = DefaultSeparator; + SeparatorBytes = DefaultSeparatorBytes; } - protected HttpHeaderParser(bool supportsMultipleValues, string separator) + protected HttpHeaderParser(bool supportsMultipleValues, string separator) : this(supportsMultipleValues) { Debug.Assert(!string.IsNullOrEmpty(separator)); + Debug.Assert(Ascii.IsValid(separator)); - _supportsMultipleValues = supportsMultipleValues; - _separator = separator; + if (supportsMultipleValues) + { + Separator = separator; + SeparatorBytes = Encoding.ASCII.GetBytes(separator); + } } // If a parser supports multiple values, a call to ParseValue/TryParseValue should return a value for 'index' diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs index cf79171bda6cf..56015f488aaae 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs @@ -258,7 +258,7 @@ public override string ToString() { // Note that if we get multiple values for a header that doesn't support multiple values, we'll // just separate the values using a comma (default separator). - string? separator = entry.Key.Parser is HttpHeaderParser parser && parser.SupportsMultipleValues ? parser.Separator : HttpHeaderParser.DefaultSeparator; + string separator = entry.Key.Separator; Debug.Assert(multiValue is not null && multiValue.Length > 0); vsb.Append(multiValue[0]); @@ -289,8 +289,7 @@ internal string GetHeaderString(HeaderDescriptor descriptor) // Note that if we get multiple values for a header that doesn't support multiple values, we'll // just separate the values using a comma (default separator). - string? separator = descriptor.Parser != null && descriptor.Parser.SupportsMultipleValues ? descriptor.Parser.Separator : HttpHeaderParser.DefaultSeparator; - return string.Join(separator, multiValue!); + return string.Join(descriptor.Separator, multiValue!); } return string.Empty; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 67e6bb5305910..68f352b11d1df 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1365,7 +1365,7 @@ private void WriteLiteralHeader(string name, ReadOnlySpan values, Encodi if (NetEventSource.Log.IsEnabled()) Trace($"{nameof(name)}={name}, {nameof(values)}={string.Join(", ", values.ToArray())}"); int bytesWritten; - while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparator, valueEncoding, headerBuffer.AvailableSpan, out bytesWritten)) + while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparatorBytes, valueEncoding, headerBuffer.AvailableSpan, out bytesWritten)) { headerBuffer.Grow(); } @@ -1373,9 +1373,9 @@ private void WriteLiteralHeader(string name, ReadOnlySpan values, Encodi headerBuffer.Commit(bytesWritten); } - private void WriteLiteralHeaderValues(ReadOnlySpan values, string? separator, Encoding? valueEncoding, ref ArrayBuffer headerBuffer) + private void WriteLiteralHeaderValues(ReadOnlySpan values, byte[]? separator, Encoding? valueEncoding, ref ArrayBuffer headerBuffer) { - if (NetEventSource.Log.IsEnabled()) Trace($"{nameof(values)}={string.Join(separator, values.ToArray())}"); + if (NetEventSource.Log.IsEnabled()) Trace($"{nameof(values)}={string.Join(Encoding.ASCII.GetString(separator ?? []), values.ToArray())}"); int bytesWritten; while (!HPackEncoder.EncodeStringLiterals(values, separator, valueEncoding, headerBuffer.AvailableSpan, out bytesWritten)) @@ -1464,19 +1464,8 @@ private int WriteHeaderCollection(HttpRequestMessage request, HttpHeaders header // For all other known headers, send them via their pre-encoded name and the associated value. WriteBytes(knownHeader.Http2EncodedName, ref headerBuffer); - string? separator = null; - if (headerValues.Length > 1) - { - HttpHeaderParser? parser = header.Key.Parser; - if (parser != null && parser.SupportsMultipleValues) - { - separator = parser.Separator; - } - else - { - separator = HttpHeaderParser.DefaultSeparator; - } - } + + byte[]? separator = headerValues.Length > 1 ? header.Key.SeparatorBytes : null; WriteLiteralHeaderValues(headerValues, separator, valueEncoding, ref headerBuffer); } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs index 8ff03d84ca67c..550b6fd53951b 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs @@ -708,19 +708,8 @@ private int BufferHeaderCollection(HttpHeaders headers) // For all other known headers, send them via their pre-encoded name and the associated value. BufferBytes(knownHeader.Http3EncodedName); - string? separator = null; - if (headerValues.Length > 1) - { - HttpHeaderParser? parser = header.Key.Parser; - if (parser != null && parser.SupportsMultipleValues) - { - separator = parser.Separator; - } - else - { - separator = HttpHeaderParser.DefaultSeparator; - } - } + + byte[]? separator = headerValues.Length > 1 ? header.Key.SeparatorBytes : null; BufferLiteralHeaderValues(headerValues, separator, valueEncoding); } @@ -728,7 +717,7 @@ private int BufferHeaderCollection(HttpHeaders headers) else { // The header is not known: fall back to just encoding the header name and value(s). - BufferLiteralHeaderWithoutNameReference(header.Key.Name, headerValues, HttpHeaderParser.DefaultSeparator, valueEncoding); + BufferLiteralHeaderWithoutNameReference(header.Key.Name, headerValues, HttpHeaderParser.DefaultSeparatorBytes, valueEncoding); } } @@ -755,7 +744,7 @@ private void BufferLiteralHeaderWithStaticNameReference(int nameIndex, string va _sendBuffer.Commit(bytesWritten); } - private void BufferLiteralHeaderWithoutNameReference(string name, ReadOnlySpan values, string separator, Encoding? valueEncoding) + private void BufferLiteralHeaderWithoutNameReference(string name, ReadOnlySpan values, byte[] separator, Encoding? valueEncoding) { int bytesWritten; while (!QPackEncoder.EncodeLiteralHeaderFieldWithoutNameReference(name, values, separator, valueEncoding, _sendBuffer.AvailableSpan, out bytesWritten)) @@ -775,7 +764,7 @@ private void BufferLiteralHeaderWithoutNameReference(string name, string value, _sendBuffer.Commit(bytesWritten); } - private void BufferLiteralHeaderValues(ReadOnlySpan values, string? separator, Encoding? valueEncoding) + private void BufferLiteralHeaderValues(ReadOnlySpan values, byte[]? separator, Encoding? valueEncoding) { int bytesWritten; while (!QPackEncoder.EncodeValueString(values, separator, valueEncoding, _sendBuffer.AvailableSpan, out bytesWritten)) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index 5fb748ae5908f..60873878d84c9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -417,16 +417,11 @@ private void WriteHeaderCollection(HttpHeaders headers, string? cookiesFromConta // Some headers such as User-Agent and Server use space as a separator (see: ProductInfoHeaderParser) if (headerValuesCount > 1) { - HttpHeaderParser? parser = header.Key.Parser; - string separator = HttpHeaderParser.DefaultSeparator; - if (parser != null && parser.SupportsMultipleValues) - { - separator = parser.Separator!; - } + byte[] separator = header.Key.SeparatorBytes; for (int i = 1; i < headerValuesCount; i++) { - WriteAsciiString(separator); + WriteBytes(separator); WriteString(headerValues[i], valueEncoding); } } diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs index 541fcb7dacd47..a73ddafe551b7 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HPack/HPackRoundtripTests.cs @@ -71,19 +71,8 @@ private static Memory HPackEncode(HttpHeaders headers, Encoding? valueEnco { // For all other known headers, send them via their pre-encoded name and the associated value. WriteBytes(knownHeader.Http2EncodedName); - string separator = null; - if (headerValuesSpan.Length > 1) - { - HttpHeaderParser parser = header.Key.Parser; - if (parser != null && parser.SupportsMultipleValues) - { - separator = parser.Separator; - } - else - { - separator = HttpHeaderParser.DefaultSeparator; - } - } + + byte[]? separator = headerValuesSpan.Length > 1 ? header.Key.SeparatorBytes : null; WriteLiteralHeaderValues(headerValuesSpan, separator); } @@ -105,7 +94,7 @@ void WriteBytes(ReadOnlySpan bytes) buffer.Commit(bytes.Length); } - void WriteLiteralHeaderValues(ReadOnlySpan values, string separator) + void WriteLiteralHeaderValues(ReadOnlySpan values, byte[]? separator) { int bytesWritten; while (!HPackEncoder.EncodeStringLiterals(values, separator, valueEncoding, buffer.AvailableSpan, out bytesWritten)) @@ -120,7 +109,7 @@ void WriteLiteralHeaderValues(ReadOnlySpan values, string separator) void WriteLiteralHeader(string name, ReadOnlySpan values) { int bytesWritten; - while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparator, valueEncoding, buffer.AvailableSpan, out bytesWritten)) + while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparatorBytes, valueEncoding, buffer.AvailableSpan, out bytesWritten)) { buffer.Grow(); FillAvailableSpaceWithOnes(buffer);