Skip to content

Commit b42c516

Browse files
authored
Deduplicate header Separator logic (#100179)
1 parent 05f5a50 commit b42c516

File tree

10 files changed

+59
-126
lines changed

10 files changed

+59
-126
lines changed

src/libraries/Common/src/System/Net/Http/aspnetcore/Http2/Hpack/HPackEncoder.cs

+10-21
Original file line numberDiff line numberDiff line change
@@ -285,12 +285,7 @@ private static bool EncodeLiteralHeaderNewNameCore(byte mask, string name, strin
285285
}
286286

287287
/// <summary>Encodes a "Literal Header Field without Indexing - New Name".</summary>
288-
public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan<string> values, string separator, Span<byte> destination, out int bytesWritten)
289-
{
290-
return EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, separator, valueEncoding: null, destination, out bytesWritten);
291-
}
292-
293-
public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan<string> values, string separator, Encoding? valueEncoding, Span<byte> destination, out int bytesWritten)
288+
public static bool EncodeLiteralHeaderFieldWithoutIndexingNewName(string name, ReadOnlySpan<string> values, byte[] separator, Encoding? valueEncoding, Span<byte> destination, out int bytesWritten)
294289
{
295290
// From https://tools.ietf.org/html/rfc7541#section-6.2.2
296291
// ------------------------------------------------------
@@ -515,12 +510,7 @@ public static bool EncodeDynamicTableSizeUpdate(int value, Span<byte> destinatio
515510
return false;
516511
}
517512

518-
public static bool EncodeStringLiterals(ReadOnlySpan<string> values, string? separator, Span<byte> destination, out int bytesWritten)
519-
{
520-
return EncodeStringLiterals(values, separator, valueEncoding: null, destination, out bytesWritten);
521-
}
522-
523-
public static bool EncodeStringLiterals(ReadOnlySpan<string> values, string? separator, Encoding? valueEncoding, Span<byte> destination, out int bytesWritten)
513+
public static bool EncodeStringLiterals(ReadOnlySpan<string> values, byte[]? separator, Encoding? valueEncoding, Span<byte> destination, out int bytesWritten)
524514
{
525515
bytesWritten = 0;
526516

@@ -536,23 +526,22 @@ public static bool EncodeStringLiterals(ReadOnlySpan<string> values, string? sep
536526
if (destination.Length != 0)
537527
{
538528
Debug.Assert(separator != null);
539-
int valueLength;
529+
Debug.Assert(Ascii.IsValid(separator));
530+
int valueLength = checked((values.Length - 1) * separator.Length);
540531

541-
// Calculate length of all parts and separators.
532+
// Calculate length of all values.
542533
if (valueEncoding is null || ReferenceEquals(valueEncoding, Encoding.Latin1))
543534
{
544-
valueLength = checked((int)(values.Length - 1) * separator.Length);
545535
foreach (string part in values)
546536
{
547-
valueLength = checked((int)(valueLength + part.Length));
537+
valueLength = checked(valueLength + part.Length);
548538
}
549539
}
550540
else
551541
{
552-
valueLength = checked((int)(values.Length - 1) * valueEncoding.GetByteCount(separator));
553542
foreach (string part in values)
554543
{
555-
valueLength = checked((int)(valueLength + valueEncoding.GetByteCount(part)));
544+
valueLength = checked(valueLength + valueEncoding.GetByteCount(part));
556545
}
557546
}
558547

@@ -571,7 +560,7 @@ public static bool EncodeStringLiterals(ReadOnlySpan<string> values, string? sep
571560

572561
for (int i = 1; i < values.Length; i++)
573562
{
574-
EncodeValueStringPart(separator, destination);
563+
separator.CopyTo(destination);
575564
destination = destination.Slice(separator.Length);
576565

577566
value = values[i];
@@ -586,8 +575,8 @@ public static bool EncodeStringLiterals(ReadOnlySpan<string> values, string? sep
586575

587576
for (int i = 1; i < values.Length; i++)
588577
{
589-
written = valueEncoding.GetBytes(separator, destination);
590-
destination = destination.Slice(written);
578+
separator.CopyTo(destination);
579+
destination = destination.Slice(separator.Length);
591580

592581
written = valueEncoding.GetBytes(values[i], destination);
593582
destination = destination.Slice(written);

src/libraries/Common/src/System/Net/Http/aspnetcore/Http3/QPack/QPackEncoder.cs

+9-19
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,9 @@ public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, str
144144
/// <summary>
145145
/// Encodes a Literal Header Field Without Name Reference, building the value by concatenating a collection of strings with separators.
146146
/// </summary>
147-
public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, ReadOnlySpan<string> values, string valueSeparator, Span<byte> destination, out int bytesWritten)
147+
public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, ReadOnlySpan<string> values, byte[] separator, Encoding? valueEncoding, Span<byte> destination, out int bytesWritten)
148148
{
149-
return EncodeLiteralHeaderFieldWithoutNameReference(name, values, valueSeparator, valueEncoding: null, destination, out bytesWritten);
150-
}
151-
152-
public static bool EncodeLiteralHeaderFieldWithoutNameReference(string name, ReadOnlySpan<string> values, string valueSeparator, Encoding? valueEncoding, Span<byte> destination, out int bytesWritten)
153-
{
154-
if (EncodeNameString(name, destination, out int nameLength) && EncodeValueString(values, valueSeparator, valueEncoding, destination.Slice(nameLength), out int valueLength))
149+
if (EncodeNameString(name, destination, out int nameLength) && EncodeValueString(values, separator, valueEncoding, destination.Slice(nameLength), out int valueLength))
155150
{
156151
bytesWritten = nameLength + valueLength;
157152
return true;
@@ -222,12 +217,7 @@ private static bool EncodeValueString(string s, Encoding? valueEncoding, Span<by
222217
/// <summary>
223218
/// Encodes a value by concatenating a collection of strings, separated by a separator string.
224219
/// </summary>
225-
public static bool EncodeValueString(ReadOnlySpan<string> values, string? separator, Span<byte> buffer, out int length)
226-
{
227-
return EncodeValueString(values, separator, valueEncoding: null, buffer, out length);
228-
}
229-
230-
public static bool EncodeValueString(ReadOnlySpan<string> values, string? separator, Encoding? valueEncoding, Span<byte> buffer, out int length)
220+
public static bool EncodeValueString(ReadOnlySpan<string> values, byte[]? separator, Encoding? valueEncoding, Span<byte> buffer, out int length)
231221
{
232222
if (values.Length == 1)
233223
{
@@ -243,18 +233,18 @@ public static bool EncodeValueString(ReadOnlySpan<string> values, string? separa
243233
if (buffer.Length > 0)
244234
{
245235
Debug.Assert(separator != null);
246-
int valueLength;
236+
Debug.Assert(Ascii.IsValid(separator));
237+
int valueLength = separator.Length * (values.Length - 1);
238+
247239
if (valueEncoding is null || ReferenceEquals(valueEncoding, Encoding.Latin1))
248240
{
249-
valueLength = separator.Length * (values.Length - 1);
250241
foreach (string part in values)
251242
{
252243
valueLength += part.Length;
253244
}
254245
}
255246
else
256247
{
257-
valueLength = valueEncoding.GetByteCount(separator) * (values.Length - 1);
258248
foreach (string part in values)
259249
{
260250
valueLength += valueEncoding.GetByteCount(part);
@@ -275,7 +265,7 @@ public static bool EncodeValueString(ReadOnlySpan<string> values, string? separa
275265

276266
for (int i = 1; i < values.Length; i++)
277267
{
278-
EncodeValueStringPart(separator, buffer);
268+
separator.CopyTo(buffer);
279269
buffer = buffer.Slice(separator.Length);
280270

281271
value = values[i];
@@ -290,8 +280,8 @@ public static bool EncodeValueString(ReadOnlySpan<string> values, string? separa
290280

291281
for (int i = 1; i < values.Length; i++)
292282
{
293-
written = valueEncoding.GetBytes(separator, buffer);
294-
buffer = buffer.Slice(written);
283+
separator.CopyTo(buffer);
284+
buffer = buffer.Slice(separator.Length);
295285

296286
written = valueEncoding.GetBytes(values[i], buffer);
297287
buffer = buffer.Slice(written);

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs

+4
Original file line numberDiff line numberDiff line change
@@ -277,5 +277,9 @@ private static bool TryDecodeUtf8(ReadOnlySpan<byte> input, [NotNullWhen(true)]
277277
decoded = null;
278278
return false;
279279
}
280+
281+
public string Separator => Parser is { } parser ? parser.Separator : HttpHeaderParser.DefaultSeparator;
282+
283+
public byte[] SeparatorBytes => Parser is { } parser ? parser.SeparatorBytes : HttpHeaderParser.DefaultSeparatorBytes;
280284
}
281285
}

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderStringValues.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ internal HeaderStringValues(HeaderDescriptor descriptor, string[] values)
4545
public override string ToString() => _value switch
4646
{
4747
string value => value,
48-
string[] values => string.Join(_header.Parser is HttpHeaderParser parser && parser.SupportsMultipleValues ? parser.Separator : HttpHeaderParser.DefaultSeparator, values),
48+
string[] values => string.Join(_header.Separator, values),
4949
_ => string.Empty,
5050
};
5151

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaderParser.cs

+17-28
Original file line numberDiff line numberDiff line change
@@ -4,53 +4,42 @@
44
using System.Collections;
55
using System.Diagnostics;
66
using System.Diagnostics.CodeAnalysis;
7+
using System.Text;
78

89
namespace System.Net.Http.Headers
910
{
1011
internal abstract class HttpHeaderParser
1112
{
12-
internal const string DefaultSeparator = ", ";
13+
public const string DefaultSeparator = ", ";
14+
public static readonly byte[] DefaultSeparatorBytes = ", "u8.ToArray();
1315

14-
private readonly bool _supportsMultipleValues;
15-
private readonly string? _separator;
16+
public bool SupportsMultipleValues { get; private set; }
1617

17-
public bool SupportsMultipleValues
18-
{
19-
get { return _supportsMultipleValues; }
20-
}
18+
public string Separator { get; private set; }
2119

22-
public string? Separator
23-
{
24-
get
25-
{
26-
Debug.Assert(_supportsMultipleValues);
27-
return _separator;
28-
}
29-
}
20+
public byte[] SeparatorBytes { get; private set; }
3021

3122
// If ValueType implements Equals() as required, there is no need to provide a comparer. A comparer is needed
3223
// e.g. if we want to compare strings using case-insensitive comparison.
33-
public virtual IEqualityComparer? Comparer
34-
{
35-
get { return null; }
36-
}
24+
public virtual IEqualityComparer? Comparer => null;
3725

3826
protected HttpHeaderParser(bool supportsMultipleValues)
3927
{
40-
_supportsMultipleValues = supportsMultipleValues;
41-
42-
if (supportsMultipleValues)
43-
{
44-
_separator = DefaultSeparator;
45-
}
28+
SupportsMultipleValues = supportsMultipleValues;
29+
Separator = DefaultSeparator;
30+
SeparatorBytes = DefaultSeparatorBytes;
4631
}
4732

48-
protected HttpHeaderParser(bool supportsMultipleValues, string separator)
33+
protected HttpHeaderParser(bool supportsMultipleValues, string separator) : this(supportsMultipleValues)
4934
{
5035
Debug.Assert(!string.IsNullOrEmpty(separator));
36+
Debug.Assert(Ascii.IsValid(separator));
5137

52-
_supportsMultipleValues = supportsMultipleValues;
53-
_separator = separator;
38+
if (supportsMultipleValues)
39+
{
40+
Separator = separator;
41+
SeparatorBytes = Encoding.ASCII.GetBytes(separator);
42+
}
5443
}
5544

5645
// If a parser supports multiple values, a call to ParseValue/TryParseValue should return a value for 'index'

src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpHeaders.cs

+2-3
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ public override string ToString()
258258
{
259259
// Note that if we get multiple values for a header that doesn't support multiple values, we'll
260260
// just separate the values using a comma (default separator).
261-
string? separator = entry.Key.Parser is HttpHeaderParser parser && parser.SupportsMultipleValues ? parser.Separator : HttpHeaderParser.DefaultSeparator;
261+
string separator = entry.Key.Separator;
262262

263263
Debug.Assert(multiValue is not null && multiValue.Length > 0);
264264
vsb.Append(multiValue[0]);
@@ -289,8 +289,7 @@ internal string GetHeaderString(HeaderDescriptor descriptor)
289289

290290
// Note that if we get multiple values for a header that doesn't support multiple values, we'll
291291
// just separate the values using a comma (default separator).
292-
string? separator = descriptor.Parser != null && descriptor.Parser.SupportsMultipleValues ? descriptor.Parser.Separator : HttpHeaderParser.DefaultSeparator;
293-
return string.Join(separator, multiValue!);
292+
return string.Join(descriptor.Separator, multiValue!);
294293
}
295294

296295
return string.Empty;

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs

+5-16
Original file line numberDiff line numberDiff line change
@@ -1365,17 +1365,17 @@ private void WriteLiteralHeader(string name, ReadOnlySpan<string> values, Encodi
13651365
if (NetEventSource.Log.IsEnabled()) Trace($"{nameof(name)}={name}, {nameof(values)}={string.Join(", ", values.ToArray())}");
13661366

13671367
int bytesWritten;
1368-
while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparator, valueEncoding, headerBuffer.AvailableSpan, out bytesWritten))
1368+
while (!HPackEncoder.EncodeLiteralHeaderFieldWithoutIndexingNewName(name, values, HttpHeaderParser.DefaultSeparatorBytes, valueEncoding, headerBuffer.AvailableSpan, out bytesWritten))
13691369
{
13701370
headerBuffer.Grow();
13711371
}
13721372

13731373
headerBuffer.Commit(bytesWritten);
13741374
}
13751375

1376-
private void WriteLiteralHeaderValues(ReadOnlySpan<string> values, string? separator, Encoding? valueEncoding, ref ArrayBuffer headerBuffer)
1376+
private void WriteLiteralHeaderValues(ReadOnlySpan<string> values, byte[]? separator, Encoding? valueEncoding, ref ArrayBuffer headerBuffer)
13771377
{
1378-
if (NetEventSource.Log.IsEnabled()) Trace($"{nameof(values)}={string.Join(separator, values.ToArray())}");
1378+
if (NetEventSource.Log.IsEnabled()) Trace($"{nameof(values)}={string.Join(Encoding.ASCII.GetString(separator ?? []), values.ToArray())}");
13791379

13801380
int bytesWritten;
13811381
while (!HPackEncoder.EncodeStringLiterals(values, separator, valueEncoding, headerBuffer.AvailableSpan, out bytesWritten))
@@ -1464,19 +1464,8 @@ private int WriteHeaderCollection(HttpRequestMessage request, HttpHeaders header
14641464

14651465
// For all other known headers, send them via their pre-encoded name and the associated value.
14661466
WriteBytes(knownHeader.Http2EncodedName, ref headerBuffer);
1467-
string? separator = null;
1468-
if (headerValues.Length > 1)
1469-
{
1470-
HttpHeaderParser? parser = header.Key.Parser;
1471-
if (parser != null && parser.SupportsMultipleValues)
1472-
{
1473-
separator = parser.Separator;
1474-
}
1475-
else
1476-
{
1477-
separator = HttpHeaderParser.DefaultSeparator;
1478-
}
1479-
}
1467+
1468+
byte[]? separator = headerValues.Length > 1 ? header.Key.SeparatorBytes : null;
14801469

14811470
WriteLiteralHeaderValues(headerValues, separator, valueEncoding, ref headerBuffer);
14821471
}

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

+5-16
Original file line numberDiff line numberDiff line change
@@ -707,27 +707,16 @@ private int BufferHeaderCollection(HttpHeaders headers)
707707

708708
// For all other known headers, send them via their pre-encoded name and the associated value.
709709
BufferBytes(knownHeader.Http3EncodedName);
710-
string? separator = null;
711-
if (headerValues.Length > 1)
712-
{
713-
HttpHeaderParser? parser = header.Key.Parser;
714-
if (parser != null && parser.SupportsMultipleValues)
715-
{
716-
separator = parser.Separator;
717-
}
718-
else
719-
{
720-
separator = HttpHeaderParser.DefaultSeparator;
721-
}
722-
}
710+
711+
byte[]? separator = headerValues.Length > 1 ? header.Key.SeparatorBytes : null;
723712

724713
BufferLiteralHeaderValues(headerValues, separator, valueEncoding);
725714
}
726715
}
727716
else
728717
{
729718
// The header is not known: fall back to just encoding the header name and value(s).
730-
BufferLiteralHeaderWithoutNameReference(header.Key.Name, headerValues, HttpHeaderParser.DefaultSeparator, valueEncoding);
719+
BufferLiteralHeaderWithoutNameReference(header.Key.Name, headerValues, HttpHeaderParser.DefaultSeparatorBytes, valueEncoding);
731720
}
732721
}
733722

@@ -754,7 +743,7 @@ private void BufferLiteralHeaderWithStaticNameReference(int nameIndex, string va
754743
_sendBuffer.Commit(bytesWritten);
755744
}
756745

757-
private void BufferLiteralHeaderWithoutNameReference(string name, ReadOnlySpan<string> values, string separator, Encoding? valueEncoding)
746+
private void BufferLiteralHeaderWithoutNameReference(string name, ReadOnlySpan<string> values, byte[] separator, Encoding? valueEncoding)
758747
{
759748
int bytesWritten;
760749
while (!QPackEncoder.EncodeLiteralHeaderFieldWithoutNameReference(name, values, separator, valueEncoding, _sendBuffer.AvailableSpan, out bytesWritten))
@@ -774,7 +763,7 @@ private void BufferLiteralHeaderWithoutNameReference(string name, string value,
774763
_sendBuffer.Commit(bytesWritten);
775764
}
776765

777-
private void BufferLiteralHeaderValues(ReadOnlySpan<string> values, string? separator, Encoding? valueEncoding)
766+
private void BufferLiteralHeaderValues(ReadOnlySpan<string> values, byte[]? separator, Encoding? valueEncoding)
778767
{
779768
int bytesWritten;
780769
while (!QPackEncoder.EncodeValueString(values, separator, valueEncoding, _sendBuffer.AvailableSpan, out bytesWritten))

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs

+2-7
Original file line numberDiff line numberDiff line change
@@ -414,16 +414,11 @@ private void WriteHeaderCollection(HttpHeaders headers, string? cookiesFromConta
414414
// Some headers such as User-Agent and Server use space as a separator (see: ProductInfoHeaderParser)
415415
if (headerValuesCount > 1)
416416
{
417-
HttpHeaderParser? parser = header.Key.Parser;
418-
string separator = HttpHeaderParser.DefaultSeparator;
419-
if (parser != null && parser.SupportsMultipleValues)
420-
{
421-
separator = parser.Separator!;
422-
}
417+
byte[] separator = header.Key.SeparatorBytes;
423418

424419
for (int i = 1; i < headerValuesCount; i++)
425420
{
426-
WriteAsciiString(separator);
421+
WriteBytes(separator);
427422
WriteString(headerValues[i], valueEncoding);
428423
}
429424
}

0 commit comments

Comments
 (0)