Fix non-thread-safe use of Random in MultipartFormDataBinaryContent (#36)

* Fix non-thread-safe use of Random in MultipartFormDataBinaryContent

MultipartFormDataBinaryContent has a static Random instance that's being used without a lock. This locks around each access (an alternative would be to use and lazily-initialize on each access a [ThreadStatic] Random).

While fixing that, also improved a few things:
- We don't need to create a char[] for the boundary values; the const string can be used directly.
- For netstandard2.0, we don't need to allocate a char[]; a stackalloc'd span can be used directly.
- For .NET, we don't need a stackalloc, either; we can use string.Create and write directly into the new string instance.
- For .NET, we don't need a dedicated Random instance, either; we can use Random.Shared.
- For .NET, Random allows writing into a byte span, so we don't need the byte[] allocation either and can stackalloc it.
- For .NET, just %'ing by the string length produces as good or better code than using a manually-computed mask.

* Address PR feedback

Author: stephentoub

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
     }