diff --git a/app/lib/frontend/handlers/custom_api.dart b/app/lib/frontend/handlers/custom_api.dart index f2cb12e4c3..0b40287646 100644 --- a/app/lib/frontend/handlers/custom_api.dart +++ b/app/lib/frontend/handlers/custom_api.dart @@ -452,10 +452,7 @@ const _commonLicenses = [ /// Handles requests for /api/search Future apiSearchHandler(shelf.Request request) async { final searchForm = SearchForm.parse(request.requestedUri.queryParameters); - final sr = await searchClient.search( - searchForm.toServiceQuery(), - sourceIp: request.sourceIp, - ); + final sr = await searchClient.search(searchForm.toServiceQuery()); final packages = sr.packageHits.map((ps) => {'package': ps.package}).toList(); final hasNextPage = sr.totalCount > searchForm.pageSize! + searchForm.offset; final result = { diff --git a/app/lib/frontend/handlers/report.dart b/app/lib/frontend/handlers/report.dart index 84740d3ebc..0bb80e4620 100644 --- a/app/lib/frontend/handlers/report.dart +++ b/app/lib/frontend/handlers/report.dart @@ -11,7 +11,6 @@ import 'package:pub_dev/admin/backend.dart'; import 'package:pub_dev/shared/configuration.dart'; import 'package:shelf/shelf.dart' as shelf; -import '../../../service/rate_limit/rate_limit.dart'; import '../../account/backend.dart'; import '../../admin/models.dart'; import '../../frontend/handlers/cache_control.dart'; @@ -25,11 +24,6 @@ import '../../shared/handlers.dart'; import '../request_context.dart'; import '../templates/report.dart'; -/// The number of requests allowed over [_reportRateLimitWindow] -const _reportRateLimit = 5; -const _reportRateLimitWindow = Duration(minutes: 10); -const _reportRateLimitWindowAsText = 'last 10 minutes'; - /// Handles GET /report Future reportPageHandler(shelf.Request request) async { final feedback = request.requestedUri.queryParameters['feedback']; @@ -175,17 +169,6 @@ Future processReportPageHandler( shelf.Request request, ReportForm form, ) async { - final sourceIp = request.sourceIp; - if (sourceIp != null) { - await verifyRequestCounts( - sourceIp: sourceIp, - operation: 'report', - limit: _reportRateLimit, - window: _reportRateLimitWindow, - windowAsText: _reportRateLimitWindowAsText, - ); - } - final now = clock.now().toUtc(); final caseId = ModerationCase.generateCaseId(now: now); diff --git a/app/lib/package/search_adapter.dart b/app/lib/package/search_adapter.dart index cb27e75210..6913adc0a1 100644 --- a/app/lib/package/search_adapter.dart +++ b/app/lib/package/search_adapter.dart @@ -62,10 +62,7 @@ class SearchAdapter { ) async { PackageSearchResult? result; try { - result = await searchClient.search( - searchForm.toServiceQuery(), - sourceIp: sourceIp, - ); + result = await searchClient.search(searchForm.toServiceQuery()); } on RateLimitException { rethrow; } catch (e, st) { diff --git a/app/lib/search/backend.dart b/app/lib/search/backend.dart index 9c0581a249..09745d4dce 100644 --- a/app/lib/search/backend.dart +++ b/app/lib/search/backend.dart @@ -495,8 +495,6 @@ class SearchBackend { // Do not cache response at the search client level, as we'll be caching // it in a processed form much longer. skipCache: true, - // Do not apply rate limit here. - sourceIp: null, ); return {'packages': rs.packageHits.map((p) => p.package).toList()}; } diff --git a/app/lib/search/search_client.dart b/app/lib/search/search_client.dart index eddc452e6f..17c1384583 100644 --- a/app/lib/search/search_client.dart +++ b/app/lib/search/search_client.dart @@ -10,7 +10,6 @@ import 'package:_pub_shared/utils/http.dart'; import 'package:clock/clock.dart'; import 'package:gcloud/service_scope.dart' as ss; -import '../../../service/rate_limit/rate_limit.dart'; import '../../account/like_backend.dart'; import '../../frontend/request_context.dart'; import '../shared/configuration.dart'; @@ -19,11 +18,6 @@ import '../shared/utils.dart'; import 'search_service.dart'; -/// The number of requests allowed over [_searchRateLimitWindow] -const _searchRateLimit = 120; -const _searchRateLimitWindow = Duration(minutes: 2); -const _searchRateLimitWindowAsText = 'last 2 minutes'; - /// Sets the search client. void registerSearchClient(SearchClient client) => ss.register(#_searchClient, client); @@ -45,7 +39,6 @@ class SearchClient { /// Calls the search service (or uses cache) to serve the [query]. Future search( ServiceSearchQuery query, { - required String? sourceIp, bool skipCache = false, }) async { // check validity first @@ -170,16 +163,6 @@ class SearchClient { ); } - if (sourceIp != null) { - await verifyRequestCounts( - sourceIp: sourceIp, - operation: 'search', - limit: _searchRateLimit, - window: _searchRateLimitWindow, - windowAsText: _searchRateLimitWindowAsText, - ); - } - if (skipCache) { return await searchFn(); } else { diff --git a/app/lib/service/rate_limit/models.dart b/app/lib/service/rate_limit/models.dart deleted file mode 100644 index a91125a62b..0000000000 --- a/app/lib/service/rate_limit/models.dart +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'package:clock/clock.dart'; -import 'package:json_annotation/json_annotation.dart'; - -import '../../../admin/actions/actions.dart'; - -part 'models.g.dart'; - -@JsonSerializable(includeIfNull: false, explicitToJson: true) -class RateLimitRequestCounter { - final DateTime started; - final int value; - - RateLimitRequestCounter({required this.started, required this.value}); - RateLimitRequestCounter.init([this.value = 0]) - : started = clock.now().toUtc(); - - factory RateLimitRequestCounter.fromJson(Map json) => - _$RateLimitRequestCounterFromJson(json); - - Map toJson() => _$RateLimitRequestCounterToJson(this); - - RateLimitRequestCounter incrementOrThrow({ - required int limit, - required Duration window, - required String windowAsText, - }) { - final now = clock.now().toUtc(); - final age = now.difference(started); - var newStarted = started; - var newValue = value + 1; - // reset the counter after the window has expired - if (age > window) { - newStarted = now; - newValue = 1; - } - // verify the counter is under the limit - if (newValue > limit) { - throw RateLimitException( - operation: 'search', - maxCount: limit, - windowAsText: windowAsText, - window: window, - ); - } - return RateLimitRequestCounter(started: newStarted, value: newValue); - } -} diff --git a/app/lib/service/rate_limit/models.g.dart b/app/lib/service/rate_limit/models.g.dart deleted file mode 100644 index f9e9d1f074..0000000000 --- a/app/lib/service/rate_limit/models.g.dart +++ /dev/null @@ -1,21 +0,0 @@ -// GENERATED CODE - DO NOT MODIFY BY HAND - -part of 'models.dart'; - -// ************************************************************************** -// JsonSerializableGenerator -// ************************************************************************** - -RateLimitRequestCounter _$RateLimitRequestCounterFromJson( - Map json, -) => RateLimitRequestCounter( - started: DateTime.parse(json['started'] as String), - value: (json['value'] as num).toInt(), -); - -Map _$RateLimitRequestCounterToJson( - RateLimitRequestCounter instance, -) => { - 'started': instance.started.toIso8601String(), - 'value': instance.value, -}; diff --git a/app/lib/service/rate_limit/rate_limit.dart b/app/lib/service/rate_limit/rate_limit.dart index 35aff05fab..e287554584 100644 --- a/app/lib/service/rate_limit/rate_limit.dart +++ b/app/lib/service/rate_limit/rate_limit.dart @@ -6,7 +6,6 @@ import 'package:clock/clock.dart'; import 'package:collection/collection.dart'; import 'package:logging/logging.dart'; import 'package:pub_dev/audit/backend.dart'; -import 'package:pub_dev/service/rate_limit/models.dart'; import '../../account/agent.dart'; import '../../audit/models.dart'; @@ -184,24 +183,3 @@ bool _containsUserId(List? users, String userId) { } return users.contains(userId); } - -Future verifyRequestCounts({ - required String sourceIp, - required String operation, - required int limit, - required Duration window, - required String windowAsText, -}) async { - final counterCacheEntry = cache.rateLimitRequestCounter(sourceIp, operation); - final cachedCounter = await counterCacheEntry.get(); - if (cachedCounter == null) { - await counterCacheEntry.set(RateLimitRequestCounter.init(1)); - } else { - final nextCounter = cachedCounter.incrementOrThrow( - limit: limit, - window: window, - windowAsText: windowAsText, - ); - await counterCacheEntry.set(nextCounter); - } -} diff --git a/app/lib/shared/redis_cache.dart b/app/lib/shared/redis_cache.dart index fe510ebae9..76a2f8f507 100644 --- a/app/lib/shared/redis_cache.dart +++ b/app/lib/shared/redis_cache.dart @@ -17,7 +17,6 @@ import 'package:neat_cache/neat_cache.dart'; import 'package:pub_dev/service/download_counts/download_counts.dart'; import '../../../service/async_queue/async_queue.dart'; -import '../../../service/rate_limit/models.dart'; import '../../../service/security_advisories/models.dart'; import '../../dartdoc/models.dart'; import '../../shared/env_config.dart'; @@ -213,24 +212,6 @@ class CachePatterns { )[url]; } - Entry rateLimitRequestCounter( - String sourceIp, - String operation, - ) { - return _cache - .withPrefix('rate-limit-request-counter/') - .withTTL(const Duration(minutes: 3)) - .withCodec(utf8) - .withCodec(json) - .withCodec( - wrapAsCodec( - encode: (RateLimitRequestCounter r) => r.toJson(), - decode: (d) => - RateLimitRequestCounter.fromJson(d as Map), - ), - )['$sourceIp/$operation']; - } - Entry scoreCardData(String package, String version) => _cache .withPrefix('scorecard-data/') .withTTL(Duration(hours: 2)) diff --git a/app/test/search/search_rate_limit_test.dart b/app/test/search/search_rate_limit_test.dart deleted file mode 100644 index e2cab6887c..0000000000 --- a/app/test/search/search_rate_limit_test.dart +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -import 'dart:async'; - -import 'package:clock/clock.dart'; -import 'package:pub_dev/frontend/handlers/listing.dart'; -import 'package:pub_dev/shared/exceptions.dart'; -import 'package:shelf/shelf.dart'; -import 'package:test/test.dart'; - -import '../frontend/handlers/_utils.dart'; -import '../shared/test_services.dart'; - -void main() { - group('Search rate limit', () { - final refTime = clock.now(); - Future search(String query, {required Duration time}) async { - return await withClock(Clock.fixed(refTime.add(time)), () async { - return await packagesHandler( - Request( - 'GET', - Uri( - scheme: 'http', - host: 'localhost', - port: 8080, // ignored - path: '/packages', - queryParameters: {'q': query}, - ), - headers: {'x-forwarded-for': '1.2.3.4'}, - ), - ); - }); - } - - testWithProfile( - 'search rate limit reached and then released', - fn: () async { - for (var i = 0; i < 120; i++) { - await expectHtmlResponse( - await search('json', time: Duration(milliseconds: i)), - ); - } - await expectLater( - search('json', time: Duration(milliseconds: 120)), - throwsA(isA()), - ); - await expectLater( - search('json', time: Duration(minutes: 2)), - throwsA(isA()), - ); - await expectHtmlResponse( - await search('json', time: Duration(minutes: 2, milliseconds: 1)), - ); - }, - ); - }); -}