Skip to content

Commit a4bc912

Browse files
gfoidlgfoidl
authored
Removed unsafe string mutation (#31850)
1 parent ca0c292 commit a4bc912

File tree

4 files changed

+55
-120
lines changed

4 files changed

+55
-120
lines changed

‎src/DataProtection/Cryptography.Internal/src/SafeHandles/BCryptAlgorithmHandle.cs‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,25 +65,22 @@ public BCryptKeyHandle GenerateSymmetricKey(byte* pbSecret, uint cbSecret)
6565
/// </summary>
6666
public string GetAlgorithmName()
6767
{
68+
const int StackAllocCharSize = 128;
69+
6870
// First, calculate how many characters are in the name.
6971
uint byteLengthOfNameWithTerminatingNull = GetProperty(Constants.BCRYPT_ALGORITHM_NAME, null, 0);
70-
CryptoUtil.Assert(byteLengthOfNameWithTerminatingNull % sizeof(char) == 0 && byteLengthOfNameWithTerminatingNull > sizeof(char), "byteLengthOfNameWithTerminatingNull % sizeof(char) == 0 && byteLengthOfNameWithTerminatingNull > sizeof(char)");
72+
CryptoUtil.Assert(byteLengthOfNameWithTerminatingNull % sizeof(char) == 0 && byteLengthOfNameWithTerminatingNull > sizeof(char) && byteLengthOfNameWithTerminatingNull <= StackAllocCharSize * sizeof(char), "byteLengthOfNameWithTerminatingNull % sizeof(char) == 0 && byteLengthOfNameWithTerminatingNull > sizeof(char) && byteLengthOfNameWithTerminatingNull <= StackAllocCharSize * sizeof(char)");
7173
uint numCharsWithoutNull = (byteLengthOfNameWithTerminatingNull - 1) / sizeof(char);
7274

7375
if (numCharsWithoutNull == 0)
7476
{
75-
return String.Empty; // degenerate case
77+
return string.Empty; // degenerate case
7678
}
7779

78-
// Allocate a string object and write directly into it (CLR team approves of this mechanism).
79-
string retVal = new String((char)0, checked((int)numCharsWithoutNull));
80-
uint numBytesCopied;
81-
fixed (char* pRetVal = retVal)
82-
{
83-
numBytesCopied = GetProperty(Constants.BCRYPT_ALGORITHM_NAME, pRetVal, byteLengthOfNameWithTerminatingNull);
84-
}
80+
char* pBuffer = stackalloc char[StackAllocCharSize];
81+
uint numBytesCopied = GetProperty(Constants.BCRYPT_ALGORITHM_NAME, pBuffer, byteLengthOfNameWithTerminatingNull);
8582
CryptoUtil.Assert(numBytesCopied == byteLengthOfNameWithTerminatingNull, "numBytesCopied == byteLengthOfNameWithTerminatingNull");
86-
return retVal;
83+
return new string(pBuffer, 0, (int)numCharsWithoutNull);
8784
}
8885

8986
/// <summary>

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

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ 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 UTF8EncodingSealed DefaultRequestHeaderEncoding = new UTF8EncodingSealed();
31-
private static readonly SpanAction<char, IntPtr> _getHeaderName = GetHeaderName;
32-
private static readonly SpanAction<char, IntPtr> _getAsciiStringNonNullCharacters = GetAsciiStringNonNullCharacters;
31+
private static readonly SpanAction<char, IntPtr> s_getHeaderName = GetHeaderName;
3332

3433
[MethodImpl(MethodImplOptions.AggressiveInlining)]
3534
private static void SetKnownMethod(ulong mask, ulong knownMethodUlong, HttpMethod knownMethod, int length)
@@ -86,15 +85,15 @@ public static unsafe string GetHeaderName(this ReadOnlySpan<byte> span)
8685

8786
fixed (byte* source = &MemoryMarshal.GetReference(span))
8887
{
89-
return string.Create(span.Length, new IntPtr(source), _getHeaderName);
88+
return string.Create(span.Length, new IntPtr(source), s_getHeaderName);
9089
}
9190
}
9291

9392
private static unsafe void GetHeaderName(Span<char> buffer, IntPtr state)
9493
{
9594
fixed (char* output = &MemoryMarshal.GetReference(buffer))
9695
{
97-
// This version if AsciiUtilities returns null if there are any null (0 byte) characters
96+
// This version of AsciiUtilities returns null if there are any null (0 byte) characters
9897
// in the string
9998
if (!StringUtilities.TryGetAsciiString((byte*)state.ToPointer(), output, buffer.Length))
10099
{
@@ -104,38 +103,11 @@ private static unsafe void GetHeaderName(Span<char> buffer, IntPtr state)
104103
}
105104

106105
public static string GetAsciiStringNonNullCharacters(this Span<byte> span)
107-
=> GetAsciiStringNonNullCharacters((ReadOnlySpan<byte>)span);
108-
109-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
110-
public static unsafe string GetAsciiStringNonNullCharacters(this ReadOnlySpan<byte> span)
111-
{
112-
if (span.IsEmpty)
113-
{
114-
return string.Empty;
115-
}
116-
117-
fixed (byte* source = &MemoryMarshal.GetReference(span))
118-
{
119-
return string.Create(span.Length, new IntPtr(source), _getAsciiStringNonNullCharacters);
120-
}
121-
}
106+
=> StringUtilities.GetAsciiStringNonNullCharacters(span);
122107

123108
public static string GetAsciiOrUTF8StringNonNullCharacters(this ReadOnlySpan<byte> span)
124109
=> StringUtilities.GetAsciiOrUTF8StringNonNullCharacters(span, DefaultRequestHeaderEncoding);
125110

126-
private static unsafe void GetAsciiStringNonNullCharacters(Span<char> buffer, IntPtr state)
127-
{
128-
fixed (char* output = &MemoryMarshal.GetReference(buffer))
129-
{
130-
// StringUtilities.TryGetAsciiString returns null if there are any null (0 byte) characters
131-
// in the string
132-
if (!StringUtilities.TryGetAsciiString((byte*)state.ToPointer(), output, buffer.Length))
133-
{
134-
throw new InvalidOperationException();
135-
}
136-
}
137-
}
138-
139111
public static string GetRequestHeaderString(this ReadOnlySpan<byte> span, string name, Func<string, Encoding?> encodingSelector)
140112
{
141113
if (ReferenceEquals(KestrelServerOptions.DefaultRequestHeaderEncodingSelector, encodingSelector))

‎src/Shared/Http2cat/Http2Utilities.cs‎

Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -144,64 +144,7 @@ public Http2Utilities(ConnectionContext clientConnectionContext, ILogger logger,
144144

145145
void IHttpHeadersHandler.OnHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
146146
{
147-
_decodedHeaders[GetAsciiStringNonNullCharacters(name)] = GetAsciiOrUTF8StringNonNullCharacters(value);
148-
}
149-
150-
public unsafe string GetAsciiStringNonNullCharacters(ReadOnlySpan<byte> span)
151-
{
152-
if (span.IsEmpty)
153-
{
154-
return string.Empty;
155-
}
156-
157-
var asciiString = new string('\0', span.Length);
158-
159-
fixed (char* output = asciiString)
160-
fixed (byte* buffer = span)
161-
{
162-
// This version if AsciiUtilities returns null if there are any null (0 byte) characters
163-
// in the string
164-
if (!StringUtilities.TryGetAsciiString(buffer, output, span.Length))
165-
{
166-
throw new InvalidOperationException();
167-
}
168-
}
169-
return asciiString;
170-
}
171-
172-
public unsafe string GetAsciiOrUTF8StringNonNullCharacters(ReadOnlySpan<byte> span)
173-
{
174-
if (span.IsEmpty)
175-
{
176-
return string.Empty;
177-
}
178-
179-
var resultString = new string('\0', span.Length);
180-
181-
fixed (char* output = resultString)
182-
fixed (byte* buffer = span)
183-
{
184-
// This version if AsciiUtilities returns null if there are any null (0 byte) characters
185-
// in the string
186-
if (!StringUtilities.TryGetAsciiString(buffer, output, span.Length))
187-
{
188-
// null characters are considered invalid
189-
if (span.IndexOf((byte)0) != -1)
190-
{
191-
throw new InvalidOperationException();
192-
}
193-
194-
try
195-
{
196-
resultString = HeaderValueEncoding.GetString(buffer, span.Length);
197-
}
198-
catch (DecoderFallbackException)
199-
{
200-
throw new InvalidOperationException();
201-
}
202-
}
203-
}
204-
return resultString;
147+
_decodedHeaders[name.GetAsciiStringNonNullCharacters()] = value.GetAsciiOrUTF8StringNonNullCharacters(HeaderValueEncoding);
205148
}
206149

207150
void IHttpHeadersHandler.OnHeadersComplete(bool endStream) { }

‎src/Shared/ServerInfrastructure/StringUtilities.cs‎

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
1717
{
1818
internal static class StringUtilities
1919
{
20-
private static readonly SpanAction<char, IntPtr> s_getAsciiOrUtf8StringNonNullCharacters = GetAsciiStringNonNullCharacters;
21-
22-
private static string GetAsciiOrUTF8StringNonNullCharacters(this Span<byte> span, Encoding defaultEncoding)
23-
=> GetAsciiOrUTF8StringNonNullCharacters((ReadOnlySpan<byte>)span, defaultEncoding);
20+
private static readonly SpanAction<char, IntPtr> s_getAsciiOrUTF8StringNonNullCharacters = GetAsciiStringNonNullCharactersWithMarker;
21+
private static readonly SpanAction<char, IntPtr> s_getAsciiStringNonNullCharacters = GetAsciiStringNonNullCharacters;
22+
private static readonly SpanAction<char, IntPtr> s_getLatin1StringNonNullCharacters = GetLatin1StringNonNullCharacters;
23+
private static readonly SpanAction<char, (string? str, char separator, uint number)> s_populateSpanWithHexSuffix = PopulateSpanWithHexSuffix;
2424

2525
public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this ReadOnlySpan<byte> span, Encoding defaultEncoding)
2626
{
@@ -31,7 +31,7 @@ public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this ReadOnlyS
3131

3232
fixed (byte* source = &MemoryMarshal.GetReference(span))
3333
{
34-
var resultString = string.Create(span.Length, new IntPtr(source), s_getAsciiOrUtf8StringNonNullCharacters);
34+
var resultString = string.Create(span.Length, (IntPtr)source, s_getAsciiOrUTF8StringNonNullCharacters);
3535

3636
// If resultString is marked, perform UTF-8 encoding
3737
if (resultString[0] == '\0')
@@ -56,11 +56,11 @@ public static unsafe string GetAsciiOrUTF8StringNonNullCharacters(this ReadOnlyS
5656
}
5757
}
5858

59-
private static unsafe void GetAsciiStringNonNullCharacters(Span<char> buffer, IntPtr state)
59+
private static unsafe void GetAsciiStringNonNullCharactersWithMarker(Span<char> buffer, IntPtr state)
6060
{
6161
fixed (char* output = &MemoryMarshal.GetReference(buffer))
6262
{
63-
// This version if AsciiUtilities returns false if there are any null ('\0') or non-Ascii
63+
// This version of AsciiUtilities returns false if there are any null ('\0') or non-Ascii
6464
// character (> 127) in the string.
6565
if (!TryGetAsciiString((byte*)state.ToPointer(), output, buffer.Length))
6666
{
@@ -70,27 +70,55 @@ private static unsafe void GetAsciiStringNonNullCharacters(Span<char> buffer, In
7070
}
7171
}
7272

73+
public static unsafe string GetAsciiStringNonNullCharacters(this ReadOnlySpan<byte> span)
74+
{
75+
if (span.IsEmpty)
76+
{
77+
return string.Empty;
78+
}
79+
80+
fixed (byte* source = &MemoryMarshal.GetReference(span))
81+
{
82+
return string.Create(span.Length, (IntPtr)source, s_getAsciiStringNonNullCharacters);
83+
}
84+
}
85+
86+
private static unsafe void GetAsciiStringNonNullCharacters(Span<char> buffer, IntPtr state)
87+
{
88+
fixed (char* output = &MemoryMarshal.GetReference(buffer))
89+
{
90+
// This version of AsciiUtilities returns false if there are any null ('\0') or non-Ascii
91+
// character (> 127) in the string.
92+
if (!TryGetAsciiString((byte*)state.ToPointer(), output, buffer.Length))
93+
{
94+
throw new InvalidOperationException();
95+
}
96+
}
97+
}
98+
7399
public static unsafe string GetLatin1StringNonNullCharacters(this ReadOnlySpan<byte> span)
74100
{
75101
if (span.IsEmpty)
76102
{
77103
return string.Empty;
78104
}
79105

80-
var resultString = new string('\0', span.Length);
106+
fixed (byte* source = &MemoryMarshal.GetReference(span))
107+
{
108+
return string.Create(span.Length, (IntPtr)source, s_getLatin1StringNonNullCharacters);
109+
}
110+
}
81111

82-
fixed (char* output = resultString)
83-
fixed (byte* buffer = span)
112+
private static unsafe void GetLatin1StringNonNullCharacters(Span<char> buffer, IntPtr state)
113+
{
114+
fixed (char* output = &MemoryMarshal.GetReference(buffer))
84115
{
85-
// This returns false if there are any null (0 byte) characters in the string.
86-
if (!TryGetLatin1String(buffer, output, span.Length))
116+
if (!TryGetLatin1String((byte*)state.ToPointer(), output, buffer.Length))
87117
{
88118
// null characters are considered invalid
89119
throw new InvalidOperationException();
90120
}
91121
}
92-
93-
return resultString;
94122
}
95123

96124
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
@@ -299,7 +327,7 @@ public static unsafe bool TryGetLatin1String(byte* input, char* output, int coun
299327
// If Vector not-accelerated or remaining less than vector size
300328
if (!Vector.IsHardwareAccelerated || input > end - Vector<sbyte>.Count)
301329
{
302-
if (IntPtr.Size == 8) // Use Intrinsic switch for branch elimination
330+
if (Environment.Is64BitProcess) // Use Intrinsic switch for branch elimination
303331
{
304332
// 64-bit: Loop longs by default
305333
while (input <= end - sizeof(long))
@@ -403,10 +431,6 @@ public static bool BytesOrdinalEqualsStringAndAscii(string previousValue, ReadOn
403431
goto NotEqual;
404432
}
405433

406-
// Use IntPtr values rather than int, to avoid unnecessary 32 -> 64 movs on 64-bit.
407-
// Unfortunately this means we also need to cast to byte* for comparisons as IntPtr doesn't
408-
// support operator comparisons (e.g. <=, >, etc).
409-
//
410434
// Note: Pointer comparison is unsigned, so we use the compare pattern (offset + length <= count)
411435
// rather than (offset <= count - length) which we'd do with signed comparison to avoid overflow.
412436
// This isn't problematic as we know the maximum length is max string length (from test above)
@@ -666,7 +690,6 @@ private static bool IsValidHeaderString(string value)
666690
return false;
667691
}
668692
}
669-
private static readonly SpanAction<char, (string? str, char separator, uint number)> s_populateSpanWithHexSuffix = PopulateSpanWithHexSuffix;
670693

671694
/// <summary>
672695
/// A faster version of String.Concat(<paramref name="str"/>, <paramref name="separator"/>, <paramref name="number"/>.ToString("X8"))

0 commit comments

Comments
 (0)