Fix non-thread-safe use of Random in MultipartFormDataBinaryContent

src/Utility/MultipartFormDataBinaryContent.cs

@@ -14,8 +14,8 @@ internal class MultipartFormDataBinaryContent : BinaryContent
 {
     private readonly MultipartFormDataContent _multipartContent;
 
-    private static Random _random = new();
-    private static readonly char[] _boundaryValues = "0123456789=ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz".ToCharArray();
+    private const int BoundaryLength = 70;
+    private const string BoundaryValues = "0123456789=ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz";
 
     public MultipartFormDataBinaryContent()
     {
@@ -120,27 +120,44 @@ public static void AddContentTypeHeader(HttpContent content, string contentType)
         content.Headers.ContentType = header;
     }
 
+#if NET6_0_OR_GREATER
+    private static string CreateBoundary() =>
+        string.Create(BoundaryLength, 0, (chars, _) =>
+        {
+            Span<byte> random = stackalloc byte[BoundaryLength];
+            Random.Shared.NextBytes(random);
+
+            for (int i = 0; i < chars.Length; i++)
+            {
+                chars[i] = BoundaryValues[random[i] % BoundaryValues.Length];
+            }
+        });
+#else
+    private static readonly Random _random = new();
+
     private static string CreateBoundary()
     {
-        Span<char> chars = new char[70];
+        Span<char> chars = stackalloc char[BoundaryLength];
 
-        byte[] random = new byte[70];
-        _random.NextBytes(random);
-
-        // The following will sample evenly from the possible values.
-        // This is important to ensuring that the odds of creating a boundary
-        // that occurs in any content part are astronomically small.
-        int mask = 255 >> 2;
+        byte[] random = new byte[BoundaryLength];
+        lock (_random)
+        {
+            _random.NextBytes(random);
+        }
 
-        Debug.Assert(_boundaryValues.Length - 1 == mask);
+        // Instead of `% BoundaryValues.Length` as is used above, use a mask to achieve the same result.
+        // `% BoundaryValues.Length` is optimized to the equivalent on .NET Core but not on .NET Framework.
+        const int Mask = 255 >> 2;
+        Debug.Assert(BoundaryValues.Length - 1 == Mask);
 
-        for (int i = 0; i < 70; i++)
+        for (int i = 0; i < chars.Length; i++)
         {
-            chars[i] = _boundaryValues[random[i] & mask];
+            chars[i] = BoundaryValues[random[i] & Mask];
         }
 
         return chars.ToString();
     }
+#endif
 
     public override bool TryComputeLength(out long length)
     {
@@ -158,22 +175,21 @@ public override bool TryComputeLength(out long length)
 
     public override void WriteTo(Stream stream, CancellationToken cancellationToken = default)
     {
-        // TODO: polyfill sync-over-async for netstandard2.0 for Azure clients.
-        // Tracked by https://github.com/Azure/azure-sdk-for-net/issues/42674
-
-#if NET6_0_OR_GREATER
+#if NET5_0_OR_GREATER
         _multipartContent.CopyTo(stream, default, cancellationToken);
 #else
-    _multipartContent.CopyToAsync(stream).GetAwaiter().GetResult();
+        // TODO: polyfill sync-over-async for netstandard2.0 for Azure clients.
+        // Tracked by https://github.com/Azure/azure-sdk-for-net/issues/42674
+        _multipartContent.CopyToAsync(stream).GetAwaiter().GetResult();
 #endif
     }
 
     public override async Task WriteToAsync(Stream stream, CancellationToken cancellationToken = default)
     {
-#if NET6_0_OR_GREATER
+#if NET5_0_OR_GREATER
         await _multipartContent.CopyToAsync(stream, cancellationToken).ConfigureAwait(false);
 #else
-    await _multipartContent.CopyToAsync(stream).ConfigureAwait(false);
+        await _multipartContent.CopyToAsync(stream).ConfigureAwait(false);
 #endif
     }