Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix pooled array leak #88810

Merged
merged 14 commits into from
Jul 18, 2023
13 changes: 12 additions & 1 deletion src/libraries/Common/tests/System/Net/Security/FakeNtlmServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace System.Net.Security
// and responses for unit test purposes. The validation checks the
// structure of the messages, their integrity and use of specified
// features (eg. MIC).
internal class FakeNtlmServer
internal class FakeNtlmServer : IDisposable
{
public FakeNtlmServer(NetworkCredential expectedCredential)
{
Expand Down Expand Up @@ -142,6 +142,14 @@ private enum AvFlags : uint
UntrustedSPN = 4,
}

public void Dispose()
{
_clientSeal?.Dispose();
_clientSeal = null;
_serverSeal?.Dispose();
_serverSeal = null;
}

private static ReadOnlySpan<byte> GetField(ReadOnlySpan<byte> payload, int fieldOffset)
{
uint offset = BinaryPrimitives.ReadUInt32LittleEndian(payload.Slice(fieldOffset + 4));
Expand Down Expand Up @@ -396,6 +404,9 @@ private void ValidateAuthentication(byte[] incomingBlob)

public void ResetKeys()
{
_clientSeal?.Dispose();
_serverSeal?.Dispose();

_clientSeal = new RC4(_clientSealingKey);
_serverSeal = new RC4(_serverSealingKey);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.ComponentModel;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Text;
Expand Down Expand Up @@ -201,32 +202,31 @@ private unsafe string GetTestDirectoryActualCasing()

if (!handle.IsInvalid)
{
const int InitialBufferSize = 4096;
char[]? buffer = ArrayPool<char>.Shared.Rent(InitialBufferSize);
uint result = GetFinalPathNameByHandle(handle, buffer);
char[] buffer = new char[4096];
uint result;
fixed (char* bufPtr = buffer)
{
result = Interop.Kernel32.GetFinalPathNameByHandle(handle, bufPtr, (uint)buffer.Length, Interop.Kernel32.FILE_NAME_NORMALIZED);
}

if (result == 0)
{
throw new Win32Exception();
}

Debug.Assert(result <= buffer.Length);

// Remove extended prefix
int skip = PathInternal.IsExtended(buffer) ? 4 : 0;

return new string(
buffer,
skip,
(int)result - skip);
return new string(buffer, skip, (int)result - skip);
}
}
catch { }

return TestDirectory;
}

private unsafe uint GetFinalPathNameByHandle(SafeFileHandle handle, char[] buffer)
{
fixed (char* bufPtr = buffer)
{
return Interop.Kernel32.GetFinalPathNameByHandle(handle, bufPtr, (uint)buffer.Length, Interop.Kernel32.FILE_NAME_NORMALIZED);
}
}

protected string CreateTestDirectory(params string[] paths)
{
string dir = Path.Combine(paths);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public sealed class MultiArrayBufferTests
[Fact]
public void BasicTest()
{
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

Assert.True(buffer.IsEmpty);
Assert.True(buffer.ActiveMemory.IsEmpty);
Expand Down Expand Up @@ -98,7 +98,7 @@ public void AddByteByByteAndConsumeByteByByte_Success()
{
const int Size = 64 * 1024 + 1;

MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

for (int i = 0; i < Size; i++)
{
Expand All @@ -124,7 +124,7 @@ public void AddSeveralBytesRepeatedlyAndConsumeSeveralBytesRepeatedly_Success()
const int ByteCount = 7;
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries

MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

for (int i = 0; i < RepeatCount; i++)
{
Expand Down Expand Up @@ -156,7 +156,7 @@ public void AddSeveralBytesRepeatedlyAndConsumeSeveralBytesRepeatedly_UsingSlice
const int ByteCount = 7;
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries

MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

for (int i = 0; i < RepeatCount; i++)
{
Expand Down Expand Up @@ -188,7 +188,7 @@ public void AddSeveralBytesRepeatedlyAndConsumeSeveralBytesRepeatedly_UsingSlice
const int ByteCount = 7;
const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries

MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

for (int i = 0; i < RepeatCount; i++)
{
Expand Down Expand Up @@ -221,7 +221,7 @@ public void CopyFromRepeatedlyAndCopyToRepeatedly_Success()

const int RepeatCount = 8 * 1024; // enough to ensure we cross several block boundaries

MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

for (int i = 0; i < RepeatCount; i++)
{
Expand Down Expand Up @@ -250,7 +250,7 @@ public void CopyFromRepeatedlyAndCopyToRepeatedly_LargeCopies_Success()

const int RepeatCount = 13;

MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

for (int i = 0; i < RepeatCount; i++)
{
Expand Down Expand Up @@ -291,7 +291,7 @@ public void EmptyMultiMemoryTest()
[Fact]
public void EnsureAvailableSpaceTest()
{
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

Assert.Equal(0, buffer.ActiveMemory.Length);
Assert.Equal(0, buffer.AvailableMemory.Length);
Expand Down Expand Up @@ -423,7 +423,7 @@ public void EnsureAvailableSpaceTest()
[Fact]
public void EnsureAvailableSpaceUpToLimitTest()
{
MultiArrayBuffer buffer = new MultiArrayBuffer(0);
using MultiArrayBuffer buffer = new MultiArrayBuffer(0);

Assert.Equal(0, buffer.ActiveMemory.Length);
Assert.Equal(0, buffer.AvailableMemory.Length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

namespace System.Net.Http.Unit.Tests.QPack
{
public class QPackDecoderTests
public class QPackDecoderTests : IDisposable
{
private const int MaxHeaderFieldSize = 8192;

Expand Down Expand Up @@ -64,6 +64,11 @@ public QPackDecoderTests()
_decoder = new QPackDecoder(MaxHeaderFieldSize);
}

public void Dispose()
{
_decoder.Dispose();
}

[Fact]
public void DecodesIndexedHeaderField_StaticTableWithValue()
{
Expand Down Expand Up @@ -318,7 +323,7 @@ private static void TestDecodeWithoutIndexing(byte[] encoded, KeyValuePair<strin

private static void TestDecode(byte[] encoded, KeyValuePair<string, string>[] expectedValues, bool expectDynamicTableEntry, int? bytesAtATime)
{
QPackDecoder decoder = new QPackDecoder(MaxHeaderFieldSize);
using QPackDecoder decoder = new QPackDecoder(MaxHeaderFieldSize);
TestHttpHeadersHandler handler = new TestHttpHeadersHandler();

// Read past header
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ public void AsSpan_ReturnsCorrectValue_DoesntClearBuilder()

Assert.NotEqual(0, sb.Length);
Assert.Equal(sb.Length, vsb.Length);
Assert.Equal(sb.ToString(), vsb.ToString());
}

[Fact]
Expand Down Expand Up @@ -275,6 +276,7 @@ public unsafe void Indexer()
Assert.Equal('b', vsb[3]);
vsb[3] = 'c';
Assert.Equal('c', vsb[3]);
vsb.Dispose();
}

[Fact]
Expand All @@ -297,6 +299,7 @@ public void EnsureCapacity_IfBufferTimesTwoWins()
builder.EnsureCapacity(33);

Assert.Equal(64, builder.Capacity);
builder.Dispose();
}

[Fact]
Expand All @@ -309,6 +312,7 @@ public void EnsureCapacity_NoAllocIfNotNeeded()
builder.EnsureCapacity(16);

Assert.Equal(64, builder.Capacity);
builder.Dispose();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ public void UnixFileModes_RestrictiveParentDir(bool overwrite)
AssertFileModeEquals(filePath, TestPermission1);
}

[Fact]
[ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
public void LinkBeforeTarget()
{
using TempDirectory source = new TempDirectory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public async Task UnixFileModes_RestrictiveParentDir_Async()
AssertFileModeEquals(filePath, TestPermission1);
}

[Fact]
[ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
public async Task LinkBeforeTargetAsync()
{
using TempDirectory source = new TempDirectory();
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/System.Memory/tests/MemoryPool/MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public static void MemoryPoolPin(int elementIndex)
public static void MemoryPoolPinBadOffset(int elementIndex)
{
MemoryPool<int> pool = MemoryPool<int>.Shared;
IMemoryOwner<int> block = pool.Rent(10);
using IMemoryOwner<int> block = pool.Rent(10);
Memory<int> memory = block.Memory;
Span<int> sp = memory.Span;
Assert.Equal(memory.Length, sp.Length);
Expand All @@ -101,7 +101,7 @@ public static void MemoryPoolPinBadOffset(int elementIndex)
public static void MemoryPoolPinOffsetAtEnd()
{
MemoryPool<int> pool = MemoryPool<int>.Shared;
IMemoryOwner<int> block = pool.Rent(10);
using IMemoryOwner<int> block = pool.Rent(10);
Memory<int> memory = block.Memory;
Span<int> sp = memory.Span;
Assert.Equal(memory.Length, sp.Length);
Expand All @@ -122,7 +122,7 @@ public static void MemoryPoolPinOffsetAtEnd()
public static void MemoryPoolPinBadOffsetTooLarge()
{
MemoryPool<int> pool = MemoryPool<int>.Shared;
IMemoryOwner<int> block = pool.Rent(10);
using IMemoryOwner<int> block = pool.Rent(10);
Memory<int> memory = block.Memory;
Span<int> sp = memory.Span;
Assert.Equal(memory.Length, sp.Length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private static async Task WriteAsyncTest(Encoding targetEncoding, string message
var model = new TestModel { Message = message };
var stream = new MemoryStream();

var transcodingStream = new TranscodingWriteStream(stream, targetEncoding);
using var transcodingStream = new TranscodingWriteStream(stream, targetEncoding);
await JsonSerializer.SerializeAsync(transcodingStream, model, model.GetType());
// The transcoding streams use Encoders and Decoders that have internal buffers. We need to flush these
// when there is no more data to be written. Stream.FlushAsync isn't suitable since it's
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ internal static async Task HandleAuthenticationRequestWithFakeServer(LoopbackSer
}
while (!isAuthenticated);

fakeNtlmServer?.Dispose();

await connection.SendResponseAsync(HttpStatusCode.OK);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ await SendMessageAsync(
else if (parts[1].Equals("GSSAPI", StringComparison.OrdinalIgnoreCase))
{
Debug.Assert(ExpectedGssapiCredential != null);
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(ExpectedGssapiCredential) { ForceNegotiateVersion = true };
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(ExpectedGssapiCredential) { ForceNegotiateVersion = true };
FakeNegotiateServer fakeNegotiateServer = new FakeNegotiateServer(fakeNtlmServer);

try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,10 @@ private static byte[] DeriveKey(ReadOnlySpan<byte> exportedSessionKey, ReadOnlyS

private void ResetKeys()
{
// Release buffers to pool
_clientSeal?.Dispose();
_serverSeal?.Dispose();

_clientSeal = new RC4(_clientSealingKey);
_serverSeal = new RC4(_serverSealingKey);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void RemoteIdentity_ThrowsOnUnauthenticated()
[ConditionalFact(nameof(IsNtlmAvailable))]
public void RemoteIdentity_ThrowsOnDisposed()
{
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
NegotiateAuthentication negotiateAuthentication = new NegotiateAuthentication(
new NegotiateAuthenticationClientOptions
{
Expand Down Expand Up @@ -98,7 +98,7 @@ public void NtlmProtocolExampleTest()
{
// Mirrors the NTLMv2 example in the NTLM specification:
NetworkCredential credential = new NetworkCredential("User", "Password", "Domain");
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(credential);
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(credential);
fakeNtlmServer.SendTimestamp = false;
fakeNtlmServer.TargetIsServer = true;
fakeNtlmServer.PreferUnicode = false;
Expand Down Expand Up @@ -151,7 +151,7 @@ public void NtlmProtocolExampleTest()
[ConditionalFact(nameof(IsNtlmAvailable))]
public void NtlmCorrectExchangeTest()
{
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
NegotiateAuthentication ntAuth = new NegotiateAuthentication(
new NegotiateAuthenticationClientOptions
{
Expand All @@ -175,7 +175,7 @@ public void NtlmCorrectExchangeTest()
[ConditionalFact(nameof(IsNtlmAvailable))]
public void NtlmIncorrectExchangeTest()
{
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
NegotiateAuthentication ntAuth = new NegotiateAuthentication(
new NegotiateAuthenticationClientOptions
{
Expand All @@ -194,7 +194,7 @@ public void NtlmIncorrectExchangeTest()
[ActiveIssue("https://github.com/dotnet/runtime/issues/65678", TestPlatforms.OSX | TestPlatforms.iOS | TestPlatforms.MacCatalyst)]
public void NtlmSignatureTest()
{
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight);
NegotiateAuthentication ntAuth = new NegotiateAuthentication(
new NegotiateAuthenticationClientOptions
{
Expand Down Expand Up @@ -250,7 +250,7 @@ private void DoNtlmExchange(FakeNtlmServer fakeNtlmServer, NegotiateAuthenticati
public void NegotiateCorrectExchangeTest(bool requestMIC, bool requestConfidentiality)
{
// Older versions of gss-ntlmssp on Linux generate MIC at incorrect offset unless ForceNegotiateVersion is specified
FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight) { ForceNegotiateVersion = true };
using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight) { ForceNegotiateVersion = true };
FakeNegotiateServer fakeNegotiateServer = new FakeNegotiateServer(fakeNtlmServer) { RequestMIC = requestMIC };
NegotiateAuthentication ntAuth = new NegotiateAuthentication(
new NegotiateAuthenticationClientOptions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ public static string TranslateWin32Expression(string? expression)
}
}

return modified ? sb.ToString() : expression;
if (!modified)
{
sb.Dispose();
return expression;
}

return sb.ToString();
}

/// <summary>Verifies whether the given Win32 expression matches the given name. Supports the following wildcards: '*', '?', '&lt;', '&gt;', '"'. The backslash character '\' escapes.</summary>
Expand Down
Loading
Loading