Skip to content

Make RateLimitingMiddleware endpoint aware #42417

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

Merged
merged 40 commits into from
Jul 12, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
ef9b7c4
Add IEndpointConventionBuilder extensions for RateLimitingMiddleware
wtgodbe May 20, 2022
df78df1
Some changes
wtgodbe Jun 2, 2022
6a4c91c
More
wtgodbe Jun 7, 2022
982cdfb
Rename
wtgodbe Jun 9, 2022
12ce9d6
More
wtgodbe Jun 21, 2022
a38aee3
Merge remote-tracking branch 'upstream/main' into wtgodbe/RateLimit2
wtgodbe Jun 21, 2022
858e7d5
More fixes
wtgodbe Jun 21, 2022
54ef30e
More stuff
wtgodbe Jun 22, 2022
b608c2d
Example of activating
wtgodbe Jun 22, 2022
e42912c
Merge remote-tracking branch 'upstream/main' into wtgodbe/RateLimit2
wtgodbe Jun 22, 2022
c15482c
Changes
wtgodbe Jun 23, 2022
ee4f648
Merge remote-tracking branch 'upstream/main' into wtgodbe/RateLimit2
wtgodbe Jun 23, 2022
e0a8c70
More stuff
wtgodbe Jun 23, 2022
7dce149
Name change
wtgodbe Jun 23, 2022
3cc73cf
Fix some stuff
wtgodbe Jun 24, 2022
ec829e6
Fixup
wtgodbe Jun 24, 2022
2fe189a
Add some tests
wtgodbe Jun 24, 2022
4e7c3a3
More tests
wtgodbe Jun 24, 2022
0cdf1fd
Renames
wtgodbe Jun 24, 2022
6389bc2
Minor
wtgodbe Jun 25, 2022
0cbbf35
Some feedback
wtgodbe Jun 27, 2022
9194a94
No more reflection
wtgodbe Jun 27, 2022
8d15768
Try to satisfy linuc
wtgodbe Jun 27, 2022
acb204d
Suppress linker warning
wtgodbe Jun 28, 2022
aeaac56
Fix linkability bug
wtgodbe Jun 28, 2022
3fb235d
Add sample project (not yet usable)
wtgodbe Jun 28, 2022
6755538
Fix sample, fix some feedback
wtgodbe Jun 29, 2022
59839bd
Feedback
wtgodbe Jun 29, 2022
23b1cc9
Merge from main
wtgodbe Jun 29, 2022
0664773
Some feedback
wtgodbe Jul 1, 2022
5ebe00b
Different key strategy
wtgodbe Jul 1, 2022
bf4f7ae
New DefaultKeyType strategy
wtgodbe Jul 1, 2022
e0a277f
HashCode.Combine
wtgodbe Jul 1, 2022
1373e87
Feedback again
wtgodbe Jul 11, 2022
286b827
Small fixes
wtgodbe Jul 11, 2022
17c496d
Merge remote-tracking branch 'upstream/main' into wtgodbe/RateLimit2
wtgodbe Jul 11, 2022
10173ec
Update compilah
wtgodbe Jul 11, 2022
a90c2f2
Feedbacks
wtgodbe Jul 11, 2022
e1871ee
Fix comment
wtgodbe Jul 11, 2022
3c3115b
Feedbock
wtgodbe Jul 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
<UsingToolNetFrameworkReferenceAssemblies Condition="'$(OS)' != 'Windows_NT'">true</UsingToolNetFrameworkReferenceAssemblies>
<!-- Disable XLIFF tasks -->
<UsingToolXliff>false</UsingToolXliff>
<!-- Use custom version of Roslyn Compiler -->
<UsingToolMicrosoftNetCompilers>true</UsingToolMicrosoftNetCompilers>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to allow for the required keyword I use in many places in this PR

</PropertyGroup>
<!--

Expand Down Expand Up @@ -194,6 +196,10 @@
framework in current .NET SDKs.
-->
<MicrosoftNETTestSdkVersion>17.1.0-preview-20211109-03</MicrosoftNETTestSdkVersion>
<!--
Use a compiler new enough to use required keyword
-->
<MicrosoftNetCompilersToolsetVersion>4.4.0-1.22315.13</MicrosoftNetCompilersToolsetVersion>
<!--
Versions of Microsoft.CodeAnalysis packages referenced by analyzers shipped in the SDK.
This need to be pinned since they're used in 3.1 apps and need to be loadable in VS 2019.
Expand Down
18 changes: 18 additions & 0 deletions src/Middleware/RateLimiting/src/AspNetKey.T.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.RateLimiting;
internal sealed class AspNetKey<TKey>: AspNetKey
{
private readonly TKey _key;

public AspNetKey(TKey key)
{
_key = key;
}

public override object? GetKey()
{
return _key;
}
}
8 changes: 8 additions & 0 deletions src/Middleware/RateLimiting/src/AspNetKey.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.RateLimiting;
internal abstract class AspNetKey
{
public abstract object? GetKey();
}
44 changes: 44 additions & 0 deletions src/Middleware/RateLimiting/src/AspNetKeyEqualityComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.RateLimiting;
internal class AspNetKeyEqualityComparer : IEqualityComparer<AspNetKey>
{
public bool Equals(AspNetKey? x, AspNetKey? y)
{
if (x == null && y == null)
{
return true;
}
else if (x == null || y == null)
{
return false;
}

var xKey = x.GetKey();
var yKey = y.GetKey();
if (xKey == null && yKey == null)
{
return true;
}
else if (xKey == null || yKey == null)
{
return false;
}

return xKey.Equals(yKey);
}

public int GetHashCode([DisallowNull] AspNetKey obj)
{
var key = obj.GetKey();
if (key is not null)
{
return key.GetHashCode();
}
// REVIEW - is this reasonable?
return default;
}
}
100 changes: 100 additions & 0 deletions src/Middleware/RateLimiting/src/AspNetLease.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.RateLimiting;

namespace Microsoft.AspNetCore.RateLimiting;
internal sealed class AspNetLease : RateLimitLease
{
private readonly RateLimitLease? _globalLease;
private readonly RateLimitLease _endpointLease;
private HashSet<string>? _metadataNames;

public AspNetLease(RateLimitLease? globalLease, RateLimitLease endpointLease)
{
_globalLease = globalLease;
_endpointLease = endpointLease;
}

public override bool IsAcquired => true;

public override IEnumerable<string> MetadataNames
{
get
{
if (_metadataNames is null)
{
_metadataNames = new HashSet<string>();
if (_globalLease is not null)
{
foreach (string metadataName in _globalLease.MetadataNames)
{
_metadataNames.Add(metadataName);
}
}
foreach (string metadataName in _endpointLease.MetadataNames)
{
_metadataNames.Add(metadataName);
}
}
return _metadataNames;
}
}

public override bool TryGetMetadata(string metadataName, out object? metadata)
{
if (_globalLease is not null)
{
// Use the first metadata item of a given name, ignore duplicates, we can't reliably merge arbitrary metadata
// Creating an object[] if there are multiple of the same metadataName could work, but makes consumption of metadata messy
// and makes MetadataName.Create<T>(...) uses no longer work
if (_globalLease.TryGetMetadata(metadataName, out metadata))
{
return true;
}
}
if (_endpointLease.TryGetMetadata(metadataName, out metadata))
{
return true;
}

metadata = null;
return false;
}

protected override void Dispose(bool disposing)
{
List<Exception>? exceptions = null;
// Dispose endpoint lease first, then global lease (reverse order of when they were acquired)
// Avoids issues where dispose might unblock a queued acquire and then the acquire fails when acquiring the next limiter.
// When disposing in reverse order there wont be any issues of unblocking an acquire that affects acquires on limiters in the chain after it

try
{
_endpointLease.Dispose();
}
catch (Exception ex)
{
exceptions ??= new List<Exception>();
exceptions.Add(ex);
}

if (_globalLease is not null)
{
try
{
_globalLease.Dispose();
}
catch (Exception ex)
{
exceptions ??= new List<Exception>();
exceptions.Add(ex);
}
}

if (exceptions is not null)
{
throw new AggregateException(exceptions);
}
}
}
25 changes: 25 additions & 0 deletions src/Middleware/RateLimiting/src/AspNetPolicy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.RateLimiting;
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.RateLimiting;
internal sealed class AspNetPolicy : IRateLimiterPolicy<AspNetKey>
{
private readonly Func<HttpContext, RateLimitPartition<AspNetKey>> _partitioner;
private readonly Func<OnRejectedContext, CancellationToken, ValueTask>? _onRejected;

public AspNetPolicy(Func<HttpContext, RateLimitPartition<AspNetKey>> partitioner, Func<OnRejectedContext, CancellationToken, ValueTask>? onRejected)
{
_partitioner = partitioner;
_onRejected = onRejected;
}

public Func<OnRejectedContext, CancellationToken, ValueTask>? OnRejected => _onRejected;

public RateLimitPartition<AspNetKey> GetPartition(HttpContext httpContext)
{
return _partitioner(httpContext);
}
}
15 changes: 15 additions & 0 deletions src/Middleware/RateLimiting/src/IRateLimiterMetadata.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.RateLimiting;

/// <summary>
/// An interface which can be used to identify a type which provides metadata needed for enabling request rate limiting support.
/// </summary>
internal interface IRateLimiterMetadata
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are IRateLimiterMetadata and RateLimiterMetadata types internal? I think at least the IRateLimiterMetadata needs to be public in order for other framework like YARP could add this metadata to their endpoints. Also using RequireRateLimiting is not an option, because IEndpointConventionBuilder is not available where YARP is creating its endpoints.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API proposal for the current implementation didn't consider this, though I think it's a good argument for making these public in the next iteration for preview7 (CC @BrennanConroy, @halter73)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We talked about this during preview6 API discussions and decided to leave it out for preview6 and talk about making it public in preview7.

{
/// <summary>
/// The name of the limiter which needs to be applied.
/// </summary>
string Name { get; }
}
24 changes: 24 additions & 0 deletions src/Middleware/RateLimiting/src/IRateLimiterPolicy.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.RateLimiting;
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.RateLimiting;

/// <summary>
/// An interface which is used to represent a RateLimiter policy.
/// </summary>
public interface IRateLimiterPolicy<TPartitionKey>
{
/// <summary>
/// Gets the <see cref="Func{OnRejectedContext, CancellationToken, ValueTask}"/> that handles requests rejected by this middleware.
/// </summary>
public Func<OnRejectedContext, CancellationToken, ValueTask>? OnRejected { get; }

/// <summary>
/// Gets the <see cref="RateLimitPartition{TPartitionKey}"/> that applies to the given <see cref="HttpContext"/>.
/// </summary>
/// <param name="httpContext">The <see cref="HttpContext"/> to get the partition for.</param>
public RateLimitPartition<TPartitionKey> GetPartition(HttpContext httpContext);
}
17 changes: 17 additions & 0 deletions src/Middleware/RateLimiting/src/LeaseContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.RateLimiting;

namespace Microsoft.AspNetCore.RateLimiting;
internal struct LeaseContext : IDisposable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about the subject but does making this struct to readonly ref improve anything!?

{
public Rejector? Rejector { get; init; }

public required RateLimitLease Lease { get; init; }

public void Dispose()
{
Lease.Dispose();
}
}
36 changes: 0 additions & 36 deletions src/Middleware/RateLimiting/src/NoLimiter.cs

This file was deleted.

23 changes: 23 additions & 0 deletions src/Middleware/RateLimiting/src/OnRejectedContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading.RateLimiting;
using Microsoft.AspNetCore.Http;

namespace Microsoft.AspNetCore.RateLimiting;

/// <summary>
/// Holds state needed for the OnRejected callback in the RateLimitingMiddleware
/// </summary>
public sealed class OnRejectedContext
{
/// <summary>
/// Gets or sets the <see cref="HttpContext"/> that the OnRejected callback will have access to
/// </summary>
public required HttpContext HttpContext { get; init; }

/// <summary>
/// Gets or sets the <see cref="RateLimitLease"/> that the OnRejected callback will have access to
/// </summary>
public required RateLimitLease Lease { get; init; }
}
22 changes: 22 additions & 0 deletions src/Middleware/RateLimiting/src/PolicyNameKey.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.AspNetCore.RateLimiting;
internal class PolicyNameKey
{
public required string PolicyName { get; init; }

public override bool Equals(object? obj)
{
if (obj is PolicyNameKey key)
{
return PolicyName.Equals(key.PolicyName);
}
return false;
}

public override int GetHashCode()
{
return PolicyName.GetHashCode();
}
}
12 changes: 12 additions & 0 deletions src/Middleware/RateLimiting/src/PolicyTypeInfo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.RateLimiting;
internal class PolicyTypeInfo
{
[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)]
public required Type PolicyType { get; init; }
public required Type PartitionKeyType { get; init; }
}
Loading