Skip to content

Commit 16bca05

Browse files
BrennanConroyBrennanConroy
authored
Use Ascii.ToUtf16 in StringUtilities (#56578)
1 parent d96d272 commit 16bca05

File tree

18 files changed

+221
-565
lines changed

18 files changed

+221
-565
lines changed

‎src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,9 @@ bool TrimAndTakeMessageHeaders(ref SequenceReader<byte> reader, bool trailers)
290290

291291
public void OnStartLine(HttpVersionAndMethod versionAndMethod, TargetOffsetPathLength targetPath, Span<byte> startLine)
292292
{
293+
// Null characters are not allowed and should have been checked by HttpParser before calling this method
294+
Debug.Assert(startLine.IndexOf((byte)0) == -1);
295+
293296
var targetStart = targetPath.Offset;
294297
// Slice out target
295298
var target = startLine[targetStart..];
@@ -322,7 +325,7 @@ public void OnStartLine(HttpVersionAndMethod versionAndMethod, TargetOffsetPathL
322325
Method = method;
323326
if (method == HttpMethod.Custom)
324327
{
325-
_methodText = startLine[..versionAndMethod.MethodEnd].GetAsciiStringNonNullCharacters();
328+
_methodText = startLine[..versionAndMethod.MethodEnd].GetAsciiString();
326329
}
327330

328331
_httpVersion = versionAndMethod.Version;
@@ -386,7 +389,7 @@ private void ParseTarget(TargetOffsetPathLength targetPath, Span<byte> target)
386389
{
387390
// The previous string does not match what the bytes would convert to,
388391
// so we will need to generate a new string.
389-
RawTarget = _parsedRawTarget = target.GetAsciiStringNonNullCharacters();
392+
RawTarget = _parsedRawTarget = target.GetAsciiString();
390393

391394
var queryLength = 0;
392395
if (target.Length == targetPath.Length)
@@ -436,7 +439,7 @@ private int ParseQuery(TargetOffsetPathLength targetPath, Span<byte> target)
436439
{
437440
// The previous string does not match what the bytes would convert to,
438441
// so we will need to generate a new string.
439-
QueryString = _parsedQueryString = query.GetAsciiStringNonNullCharacters();
442+
QueryString = _parsedQueryString = query.GetAsciiString();
440443
}
441444
else
442445
{
@@ -482,7 +485,7 @@ private void OnAuthorityFormTarget(HttpMethod method, Span<byte> target)
482485
{
483486
// The previous string does not match what the bytes would convert to,
484487
// so we will need to generate a new string.
485-
RawTarget = _parsedRawTarget = target.GetAsciiStringNonNullCharacters();
488+
RawTarget = _parsedRawTarget = target.GetAsciiString();
486489
}
487490
else
488491
{
@@ -542,7 +545,7 @@ private void OnAbsoluteFormTarget(TargetOffsetPathLength targetPath, Span<byte>
542545
{
543546
// The previous string does not match what the bytes would convert to,
544547
// so we will need to generate a new string.
545-
RawTarget = _parsedRawTarget = target.GetAsciiStringNonNullCharacters();
548+
RawTarget = _parsedRawTarget = target.GetAsciiString();
546549
}
547550
catch (InvalidOperationException)
548551
{
@@ -572,7 +575,7 @@ private void OnAbsoluteFormTarget(TargetOffsetPathLength targetPath, Span<byte>
572575
{
573576
// The previous string does not match what the bytes would convert to,
574577
// so we will need to generate a new string.
575-
QueryString = _parsedQueryString = query.GetAsciiStringNonNullCharacters();
578+
QueryString = _parsedQueryString = query.GetAsciiString();
576579
}
577580
else
578581
{

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpHeaders.Generated.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7481,7 +7481,7 @@ internal void ClearPseudoRequestHeaders()
74817481
}
74827482

74837483
[MethodImpl(MethodImplOptions.AggressiveInlining)]
7484-
internal static unsafe ushort ReadUnalignedLittleEndian_ushort(ref byte source)
7484+
internal static ushort ReadUnalignedLittleEndian_ushort(ref byte source)
74857485
{
74867486
ushort result = Unsafe.ReadUnaligned<ushort>(ref source);
74877487
if (!BitConverter.IsLittleEndian)
@@ -7491,7 +7491,7 @@ internal static unsafe ushort ReadUnalignedLittleEndian_ushort(ref byte source)
74917491
return result;
74927492
}
74937493
[MethodImpl(MethodImplOptions.AggressiveInlining)]
7494-
internal static unsafe uint ReadUnalignedLittleEndian_uint(ref byte source)
7494+
internal static uint ReadUnalignedLittleEndian_uint(ref byte source)
74957495
{
74967496
uint result = Unsafe.ReadUnaligned<uint>(ref source);
74977497
if (!BitConverter.IsLittleEndian)
@@ -7501,7 +7501,7 @@ internal static unsafe uint ReadUnalignedLittleEndian_uint(ref byte source)
75017501
return result;
75027502
}
75037503
[MethodImpl(MethodImplOptions.AggressiveInlining)]
7504-
internal static unsafe ulong ReadUnalignedLittleEndian_ulong(ref byte source)
7504+
internal static ulong ReadUnalignedLittleEndian_ulong(ref byte source)
75057505
{
75067506
ulong result = Unsafe.ReadUnaligned<ulong>(ref source);
75077507
if (!BitConverter.IsLittleEndian)
@@ -7511,11 +7511,11 @@ internal static unsafe ulong ReadUnalignedLittleEndian_ulong(ref byte source)
75117511
return result;
75127512
}
75137513
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
7514-
public unsafe void Append(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value, bool checkForNewlineChars)
7514+
public void Append(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value, bool checkForNewlineChars)
75157515
{
75167516
ref byte nameStart = ref MemoryMarshal.GetReference(name);
75177517
var nameStr = string.Empty;
7518-
ref StringValues values = ref Unsafe.AsRef<StringValues>(null);
7518+
ref StringValues values = ref Unsafe.NullRef<StringValues>();
75197519
var flag = 0L;
75207520

75217521
// Does the name match any "known" headers
@@ -7924,9 +7924,9 @@ public unsafe void Append(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value, boo
79247924
}
79257925

79267926
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
7927-
public unsafe bool TryHPackAppend(int index, ReadOnlySpan<byte> value, bool checkForNewlineChars)
7927+
public bool TryHPackAppend(int index, ReadOnlySpan<byte> value, bool checkForNewlineChars)
79287928
{
7929-
ref StringValues values = ref Unsafe.AsRef<StringValues>(null);
7929+
ref StringValues values = ref Unsafe.NullRef<StringValues>();
79307930
var nameStr = string.Empty;
79317931
var flag = 0L;
79327932

@@ -8136,9 +8136,9 @@ public unsafe bool TryHPackAppend(int index, ReadOnlySpan<byte> value, bool chec
81368136
}
81378137

81388138
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
8139-
public unsafe bool TryQPackAppend(int index, ReadOnlySpan<byte> value, bool checkForNewlineChars)
8139+
public bool TryQPackAppend(int index, ReadOnlySpan<byte> value, bool checkForNewlineChars)
81408140
{
8141-
ref StringValues values = ref Unsafe.AsRef<StringValues>(null);
8141+
ref StringValues values = ref Unsafe.NullRef<StringValues>();
81428142
var nameStr = string.Empty;
81438143
var flag = 0L;
81448144

@@ -14806,7 +14806,7 @@ internal void ClearInvalidH2H3Headers()
1480614806
{
1480714807
_bits &= ~13161725953;
1480814808
}
14809-
internal unsafe void CopyToFast(ref BufferWriter<PipeWriter> output)
14809+
internal void CopyToFast(ref BufferWriter<PipeWriter> output)
1481014810
{
1481114811
var tempBits = (ulong)_bits;
1481214812
// Set exact next
@@ -14823,7 +14823,7 @@ internal unsafe void CopyToFast(ref BufferWriter<PipeWriter> output)
1482314823
return;
1482414824
}
1482514825

14826-
ref readonly StringValues values = ref Unsafe.AsRef<StringValues>(null);
14826+
ref readonly StringValues values = ref Unsafe.NullRef<StringValues>();
1482714827
do
1482814828
{
1482914829
int keyStart;
@@ -17629,4 +17629,4 @@ public bool MoveNext()
1762917629
}
1763017630
}
1763117631
}
17632-
}
17632+
}

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpParser.cs‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,24 +54,34 @@ internal HttpParser(bool showErrorDetails, bool disableHttp1LineFeedTerminators)
5454
private const byte ByteQuestionMark = (byte)'?';
5555
private const byte BytePercentage = (byte)'%';
5656
private const int MinTlsRequestSize = 1; // We need at least 1 byte to check for a proper TLS request line
57+
private static ReadOnlySpan<byte> RequestLineDelimiters => [ByteLF, 0];
5758

5859
/// <summary>
5960
/// This API supports framework infrastructure and is not intended to be used
6061
/// directly from application code.
6162
/// </summary>
6263
public bool ParseRequestLine(TRequestHandler handler, ref SequenceReader<byte> reader)
6364
{
64-
if (reader.TryReadTo(out ReadOnlySpan<byte> requestLine, ByteLF, advancePastDelimiter: true))
65+
// Find the next delimiter.
66+
if (!reader.TryReadToAny(out ReadOnlySpan<byte> requestLine, RequestLineDelimiters, advancePastDelimiter: false))
6567
{
66-
ParseRequestLine(handler, requestLine);
67-
return true;
68+
return false;
6869
}
6970

70-
return false;
71-
}
71+
// Consume the delimiter.
72+
var foundDelimiter = reader.TryRead(out var next);
73+
Debug.Assert(foundDelimiter);
74+
// If null character found, or request line is empty
75+
if (next == 0 || requestLine.Length == 0)
76+
{
77+
// Rewind and re-read to format error message correctly
78+
reader.Rewind(requestLine.Length + 1);
79+
var readResult = reader.TryReadExact(requestLine.Length + 1, out var requestLineSequence);
80+
Debug.Assert(readResult);
81+
requestLine = requestLineSequence.IsSingleSegment ? requestLineSequence.FirstSpan : requestLineSequence.ToArray();
82+
RejectRequestLine(requestLine);
83+
}
7284

73-
private void ParseRequestLine(TRequestHandler handler, ReadOnlySpan<byte> requestLine)
74-
{
7585
// Get Method and set the offset
7686
var method = requestLine.GetKnownMethod(out var methodEnd);
7787
if (method == HttpMethod.Custom)
@@ -175,6 +185,8 @@ private void ParseRequestLine(TRequestHandler handler, ReadOnlySpan<byte> reques
175185
// in-place normalization and decoding to transform into a canonical path
176186
var startLine = MemoryMarshal.CreateSpan(ref MemoryMarshal.GetReference(requestLine), queryEnd);
177187
handler.OnStartLine(versionAndMethod, path, startLine);
188+
189+
return true;
178190
}
179191

180192
/// <summary>

‎src/Servers/Kestrel/Core/src/Internal/Http/HttpRequestHeaders.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ private bool AddValueUnknown(string key, StringValues value)
147147
}
148148

149149
[MethodImpl(MethodImplOptions.NoInlining)]
150-
private unsafe void AppendUnknownHeaders(string name, string valueString)
150+
private void AppendUnknownHeaders(string name, string valueString)
151151
{
152152
name = GetInternedHeaderName(name);
153153
Unknown.TryGetValue(name, out var existing);

‎src/Servers/Kestrel/Core/src/Internal/Http/PathDecoder.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,6 @@ public static string DecodePath(Span<byte> path, bool pathEncoded, string rawTar
4242
return rawTarget;
4343
}
4444

45-
return path.Slice(0, pathLength).GetAsciiStringNonNullCharacters();
45+
return path.Slice(0, pathLength).GetAsciiString();
4646
}
4747
}

‎src/Servers/Kestrel/Core/src/Internal/Infrastructure/HttpUtilities.cs‎

Lines changed: 41 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ internal static partial class HttpUtilities
2828
private const ulong _http11VersionLong = 3543824036068086856; // GetAsciiStringAsLong("HTTP/1.1"); const results in better codegen
2929

3030
private static readonly UTF8Encoding DefaultRequestHeaderEncoding = new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true);
31-
private static readonly SpanAction<char, IntPtr> s_getHeaderName = GetHeaderName;
3231

3332
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3433
private static void SetKnownMethod(ulong mask, ulong knownMethodUlong, HttpMethod knownMethod, int length)
@@ -81,72 +80,89 @@ private static ulong GetMaskAsLong(ReadOnlySpan<byte> bytes)
8180
return BinaryPrimitives.ReadUInt64LittleEndian(bytes);
8281
}
8382

84-
// The same as GetAsciiStringNonNullCharacters but throws BadRequest
83+
// The same as GetAsciiString but throws BadRequest for null character
8584
[MethodImpl(MethodImplOptions.AggressiveInlining)]
86-
public static unsafe string GetHeaderName(this ReadOnlySpan<byte> span)
85+
public static string GetHeaderName(this ReadOnlySpan<byte> span)
8786
{
8887
if (span.IsEmpty)
8988
{
9089
return string.Empty;
9190
}
9291

93-
fixed (byte* source = &MemoryMarshal.GetReference(span))
92+
var str = string.Create(span.Length, span, static (destination, source) =>
9493
{
95-
return string.Create(span.Length, new IntPtr(source), s_getHeaderName);
96-
}
97-
}
98-
99-
private static unsafe void GetHeaderName(Span<char> buffer, IntPtr state)
100-
{
101-
fixed (char* output = &MemoryMarshal.GetReference(buffer))
102-
{
103-
// This version of AsciiUtilities returns null if there are any null (0 byte) characters
104-
// in the string
105-
if (!StringUtilities.TryGetAsciiString((byte*)state.ToPointer(), output, buffer.Length))
94+
if (source.Contains((byte)0)
95+
|| Ascii.ToUtf16(source, destination, out var written) != OperationStatus.Done)
10696
{
10797
KestrelBadHttpRequestException.Throw(RequestRejectionReason.InvalidCharactersInHeaderName);
10898
}
109-
}
99+
else
100+
{
101+
Debug.Assert(written == destination.Length);
102+
}
103+
});
104+
105+
return str;
110106
}
111107

112-
public static string GetAsciiStringNonNullCharacters(this Span<byte> span)
113-
=> StringUtilities.GetAsciiStringNonNullCharacters(span);
108+
// Null checks must be done independently of this method (if required)
109+
public static string GetAsciiString(this Span<byte> span)
110+
=> StringUtilities.GetAsciiString(span);
114111

115-
public static string GetAsciiOrUTF8StringNonNullCharacters(this ReadOnlySpan<byte> span)
116-
=> StringUtilities.GetAsciiOrUTF8StringNonNullCharacters(span, DefaultRequestHeaderEncoding);
112+
// Null checks must be done independently of this method (if required)
113+
public static string GetAsciiOrUTF8String(this ReadOnlySpan<byte> span)
114+
=> StringUtilities.GetAsciiOrUTF8String(span, DefaultRequestHeaderEncoding);
117115

118116
public static string GetRequestHeaderString(this ReadOnlySpan<byte> span, string name, Func<string, Encoding?> encodingSelector, bool checkForNewlineChars)
119117
{
120118
string result;
121119
if (ReferenceEquals(KestrelServerOptions.DefaultHeaderEncodingSelector, encodingSelector))
122120
{
123-
result = span.GetAsciiOrUTF8StringNonNullCharacters(DefaultRequestHeaderEncoding);
121+
result = span.GetAsciiOrUTF8String(DefaultRequestHeaderEncoding);
124122
}
125123
else
126124
{
127125
result = span.GetRequestHeaderStringWithoutDefaultEncodingCore(name, encodingSelector);
128126
}
129127

130128
// New Line characters (CR, LF) are considered invalid at this point.
131-
if (checkForNewlineChars && ((ReadOnlySpan<char>)result).IndexOfAny('\r', '\n') >= 0)
129+
// Null characters are also not allowed.
130+
var hasInvalidChar = checkForNewlineChars ?
131+
((ReadOnlySpan<char>)result).ContainsAny('\r', '\n', '\0')
132+
: ((ReadOnlySpan<char>)result).Contains('\0');
133+
134+
if (hasInvalidChar)
132135
{
133-
throw new InvalidOperationException("Newline characters (CR/LF) are not allowed in request headers.");
136+
ThrowForInvalidCharacter(checkForNewlineChars);
134137
}
135138

136139
return result;
137140
}
138141

142+
[MethodImpl(MethodImplOptions.NoInlining)]
143+
private static void ThrowForInvalidCharacter(bool checkForNewlines)
144+
{
145+
if (checkForNewlines)
146+
{
147+
throw new InvalidOperationException("Newline characters (CR/LF) or Null are not allowed in request headers.");
148+
}
149+
else
150+
{
151+
throw new InvalidOperationException("Null characters are not allowed in request headers.");
152+
}
153+
}
154+
139155
private static string GetRequestHeaderStringWithoutDefaultEncodingCore(this ReadOnlySpan<byte> span, string name, Func<string, Encoding?> encodingSelector)
140156
{
141157
var encoding = encodingSelector(name);
142158

143159
if (encoding is null)
144160
{
145-
return span.GetAsciiOrUTF8StringNonNullCharacters(DefaultRequestHeaderEncoding);
161+
return span.GetAsciiOrUTF8String(DefaultRequestHeaderEncoding);
146162
}
147163
if (ReferenceEquals(encoding, Encoding.Latin1))
148164
{
149-
return span.GetLatin1StringNonNullCharacters();
165+
return span.GetLatin1String();
150166
}
151167
try
152168
{

0 commit comments

Comments
 (0)