Fix rate limit cache key. (#7598)

dart-lang · Mar 27, 2024 · 7232d49 · 7232d49
1 parent 5e8a2ef
commit 7232d49
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 7 deletions.
diff --git a/app/lib/service/rate_limit/rate_limit.dart b/app/lib/service/rate_limit/rate_limit.dart
@@ -83,13 +83,6 @@ Future<void> _verifyRateLimit({
     return;
   }
 
-  final cacheEntryKey = [
-    rateLimit.operation,
-    rateLimit.scope.name,
-    if (package != null) 'package-$package',
-    if (agentId != null) 'agentId-$agentId',
-  ].join('/');
-
   List<AuditLogRecord>? auditEntriesFromLastDay;
 
   Future<void> check({
@@ -102,6 +95,14 @@ Future<void> _verifyRateLimit({
       return;
     }
 
+    final cacheEntryKey = [
+      rateLimit.operation,
+      rateLimit.scope.name,
+      if (package != null) 'package-$package',
+      if (agentId != null) 'agentId-$agentId',
+      window.inSeconds,
+    ].join('/');
+
     final entry = cache.rateLimitUntil(cacheEntryKey);
     final current = await entry.get();
     if (current != null && current.isAfter(clock.now())) {

diff --git a/app/test/package/upload_rate_limit_test.dart b/app/test/package/upload_rate_limit_test.dart
@@ -166,6 +166,47 @@ environment:
       },
     );
 
+    testWithProfile(
+      'retried failures report the correct window',
+      testProfile: TestProfile(packages: [], defaultUser: adminAtPubDevEmail),
+      fn: () async {
+        await _withRateLimits(
+          [
+            RateLimit(
+              operation: AuditLogRecordKind.packagePublished,
+              scope: RateLimitScope.package,
+              burst: 2,
+              hourly: 4,
+              daily: 24,
+            ),
+          ],
+          () async {
+            for (var i = 0; i < 24; i++) {
+              await upload(version: '1.0.$i', time: Duration(minutes: i * 60));
+            }
+
+            for (var i = 0; i < 10; i++) {
+              final rs = upload(
+                  version: '1.0.24',
+                  time: Duration(minutes: 23 * 60, seconds: 5));
+              await expectLater(
+                rs,
+                throwsA(
+                  isA<RateLimitException>().having(
+                    (e) => e.message,
+                    'message',
+                    contains('(24 in the last day)'),
+                  ),
+                ),
+              );
+            }
+            await upload(
+                version: '1.0.24', time: Duration(days: 1, minutes: 1));
+          },
+        );
+      },
+    );
+
     testWithProfile(
       'new packages 1 per minute',
       testProfile: TestProfile(packages: [], defaultUser: adminAtPubDevEmail),