From 189767fbafadd6569f165b1422e88c1dc5f93b1e Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Mon, 18 Apr 2022 15:42:47 +0200 Subject: [PATCH 1/2] fix: gateway use cf dns to prevent rate limit instead of durable object --- .../durable-objects/gateway-rate-limits.js | 122 ------------------ packages/edge-gateway/src/env.js | 3 - packages/edge-gateway/src/gateway.js | 43 +----- packages/edge-gateway/src/index.js | 1 - packages/edge-gateway/test/rate-limit.spec.js | 60 --------- packages/edge-gateway/wrangler.toml | 16 +-- 6 files changed, 8 insertions(+), 237 deletions(-) delete mode 100644 packages/edge-gateway/src/durable-objects/gateway-rate-limits.js delete mode 100644 packages/edge-gateway/test/rate-limit.spec.js diff --git a/packages/edge-gateway/src/durable-objects/gateway-rate-limits.js b/packages/edge-gateway/src/durable-objects/gateway-rate-limits.js deleted file mode 100644 index 26026f4..0000000 --- a/packages/edge-gateway/src/durable-objects/gateway-rate-limits.js +++ /dev/null @@ -1,122 +0,0 @@ -/** - * @typedef {Object} RateLimitResponse - * @property {Record} shouldPreventRateLimit whether request should be prevented per gateway - * - * @typedef {Object} RateLimitCharacteristics - * @property {number} RATE_LIMIT_REQUESTS - * @property {number} RATE_LIMIT_TIMEFRAME - */ - -/** - * Durable Object to keep track of gateway rating limits. - * State: number[] - */ -export class GatewayRateLimits4 { - constructor(state, env) { - this.state = state - this.id = this.state.id.name - - /** @type {Array} */ - this.ipfsGateways = JSON.parse(env.IPFS_GATEWAYS) - // `blockConcurrencyWhile()` ensures no requests are delivered until initialization completes. - this.state.blockConcurrencyWhile(async () => { - // Get state and initialize if not existing - /** @type {Map>} */ - this.gatewayUsage = - (await this.state.storage.get(this.id)) || this._createGatewayUsage() - }) - } - - // Handle HTTP requests from clients. - async fetch(request) { - // Apply requested action. - let url = new URL(request.url) - switch (url.pathname) { - case '/request': - const shouldPreventRateLimit = {} - this.ipfsGateways.forEach((gwUrl) => { - shouldPreventRateLimit[gwUrl] = this._shouldPreventRequest(gwUrl) - }) - - await this.state.storage.put(this.id, this.gatewayUsage, { - allowUnconfirmed: true, - }) - - return new Response(JSON.stringify(shouldPreventRateLimit)) - default: - return new Response('Not found', { status: 404 }) - } - } - - /** - * @param {string} gatewayUrl - */ - _shouldPreventRequest(gatewayUrl) { - const rateLimitConfig = getRateLimitConfig(gatewayUrl) - if (rateLimitConfig.RATE_LIMIT_REQUESTS === Infinity) { - return false - } - - // Filter out outdated requests - const now = Date.now() - const rateLimitUsage = this.gatewayUsage - .get(gatewayUrl) - .filter((ts) => now - ts <= rateLimitConfig.RATE_LIMIT_TIMEFRAME) - - // Add new request to track - if (rateLimitUsage.length < rateLimitConfig.RATE_LIMIT_REQUESTS) { - rateLimitUsage.push(Date.now()) - this.gatewayUsage.set(gatewayUrl, rateLimitUsage) - return false - } - this.gatewayUsage.set(gatewayUrl, rateLimitUsage) - - return true - } - - /** - * @return {Map>} - */ - _createGatewayUsage() { - const gatewayUsage = new Map() - this.ipfsGateways.forEach((gwUrl) => { - gatewayUsage.set(gwUrl, []) - }) - return gatewayUsage - } -} - -const SECOND = 1000 -const MINUTE = SECOND * 60 - -/** - * Get rate limiting characteristics of a given Gateway. - * - * @param {string} gatewayUrl - * @return {RateLimitCharacteristics} - */ -function getRateLimitConfig(gatewayUrl) { - switch (gatewayUrl) { - case 'https://ipfs.io': - return { - RATE_LIMIT_REQUESTS: Infinity, - RATE_LIMIT_TIMEFRAME: SECOND * 10, - } - case 'https://cf.nftstorage.link': - return { - RATE_LIMIT_REQUESTS: Infinity, - RATE_LIMIT_TIMEFRAME: SECOND * 10, - } - case 'https://nft-storage.mypinata.cloud/': - return { - RATE_LIMIT_REQUESTS: 400, - RATE_LIMIT_TIMEFRAME: MINUTE, - } - // Default to 100 - default: - return { - RATE_LIMIT_REQUESTS: 100, - RATE_LIMIT_TIMEFRAME: MINUTE, - } - } -} diff --git a/packages/edge-gateway/src/env.js b/packages/edge-gateway/src/env.js index 1469424..f8ee49a 100644 --- a/packages/edge-gateway/src/env.js +++ b/packages/edge-gateway/src/env.js @@ -17,7 +17,6 @@ import { Logging } from './logs.js' * @property {Object} GATEWAYMETRICS * @property {Object} SUMMARYMETRICS * @property {Object} CIDSTRACKER - * @property {Object} GATEWAYRATELIMITS * @property {Object} GATEWAYREDIRECTCOUNTER * @property {KVNamespace} DENYLIST * @@ -28,7 +27,6 @@ import { Logging } from './logs.js' * @property {DurableObjectNamespace} gatewayMetricsDurable * @property {DurableObjectNamespace} summaryMetricsDurable * @property {DurableObjectNamespace} cidsTrackerDurable - * @property {DurableObjectNamespace} gatewayRateLimitsDurable * @property {DurableObjectNamespace} gatewayRedirectCounter * @property {number} REQUEST_TIMEOUT * @property {Toucan} [sentry] @@ -48,7 +46,6 @@ export function envAll(request, env, ctx) { env.gatewayMetricsDurable = env.GATEWAYMETRICS env.summaryMetricsDurable = env.SUMMARYMETRICS env.cidsTrackerDurable = env.CIDSTRACKER - env.gatewayRateLimitsDurable = env.GATEWAYRATELIMITS env.gatewayRedirectCounter = env.GATEWAYREDIRECTCOUNTER env.REQUEST_TIMEOUT = env.REQUEST_TIMEOUT || 20000 env.IPFS_GATEWAY_HOSTNAME = env.GATEWAY_HOSTNAME diff --git a/packages/edge-gateway/src/gateway.js b/packages/edge-gateway/src/gateway.js index 33a78d8..1b86678 100644 --- a/packages/edge-gateway/src/gateway.js +++ b/packages/edge-gateway/src/gateway.js @@ -74,12 +74,10 @@ export async function gatewayGet(request, env, ctx) { } // Prepare IPFS gateway requests - const shouldPreventRateLimit = await getGatewayRateLimitState(request, env) const gatewayReqs = env.ipfsGateways.map((gwUrl) => gatewayFetch(gwUrl, cid, request, { pathname, timeout: env.REQUEST_TIMEOUT, - shouldPreventRateLimit: shouldPreventRateLimit[gwUrl], }) ) try { @@ -194,24 +192,13 @@ async function storeWinnerGwResponse(request, env, winnerGwResponse) { * @param {Object} [options] * @param {string} [options.pathname] * @param {number} [options.timeout] - * @param {boolean} [options.shouldPreventRateLimit] */ async function gatewayFetch( gwUrl, cid, request, - { pathname = '', timeout = 60000, shouldPreventRateLimit = false } = {} + { pathname = '', timeout = 60000 } = {} ) { - // Block before hitting rate limit if needed - if (shouldPreventRateLimit) { - /** @type {GatewayResponse} */ - return { - url: gwUrl, - aborted: true, - reason: REQUEST_PREVENTED_RATE_LIMIT_CODE, - } - } - const ipfsUrl = new URL('ipfs', gwUrl) const controller = new AbortController() const startTs = Date.now() @@ -293,34 +280,6 @@ async function updateGatewayRedirectCounter(request, env) { await stub.fetch(getDurableRequestUrl(request, 'update')) } -/** - * @param {Request} request - * @param {import('./env').Env} env - */ -async function getGatewayRateLimitState(request, env) { - // Get durable object for gateway rate limits - const id = env.gatewayRateLimitsDurable.idFromName(GATEWAY_RATE_LIMIT_ID) - const stub = env.gatewayRateLimitsDurable.get(id) - - try { - const stubResponse = await stub.fetch( - getDurableRequestUrl(request, 'request') - ) - - /** @type {import('./durable-objects/gateway-rate-limits').RateLimitResponse} */ - const rateLimitResponse = await stubResponse.json() - return rateLimitResponse - } catch (err) { - env.log.log(err, 'error') - // Just force no prevention of rate limit - const shouldPreventRateLimit = {} - env.ipfsGateways.forEach((gwUrl) => { - shouldPreventRateLimit[gwUrl] = false - }) - return shouldPreventRateLimit - } -} - /** * @param {Request} request * @param {import('./env').Env} env diff --git a/packages/edge-gateway/src/index.js b/packages/edge-gateway/src/index.js index 82ec704..3608f87 100644 --- a/packages/edge-gateway/src/index.js +++ b/packages/edge-gateway/src/index.js @@ -11,7 +11,6 @@ import { metricsGet } from './metrics.js' export { GatewayMetrics0 } from './durable-objects/gateway-metrics.js' export { SummaryMetrics0 } from './durable-objects/summary-metrics.js' export { CidsTracker0 } from './durable-objects/cids.js' -export { GatewayRateLimits4 } from './durable-objects/gateway-rate-limits.js' export { GatewayRedirectCounter0 } from './durable-objects/gateway-redirect-counter.js' import { addCorsHeaders, withCorsHeaders } from './cors.js' diff --git a/packages/edge-gateway/test/rate-limit.spec.js b/packages/edge-gateway/test/rate-limit.spec.js deleted file mode 100644 index 0854869..0000000 --- a/packages/edge-gateway/test/rate-limit.spec.js +++ /dev/null @@ -1,60 +0,0 @@ -import test from 'ava' - -import { GATEWAY_RATE_LIMIT_ID } from '../src/constants.js' -import { gateways } from './constants.js' -import { getMiniflare, getGatewayRateLimitsName } from './utils.js' - -test.beforeEach((t) => { - // Create a new Miniflare environment for each test - t.context = { - mf: getMiniflare(), - } -}) - -test('Receives falsy on should block when not on high load', async (t) => { - const { mf } = t.context - const cid = 'bafkreidyeivj7adnnac6ljvzj2e3rd5xdw3revw4da7mx2ckrstapoupoq' - - const p = await mf.dispatchFetch(`https://${cid}.ipfs.localhost:8787`) - await p.waitUntil() - - const ns = await mf.getDurableObjectNamespace(getGatewayRateLimitsName()) - const id = ns.idFromName(GATEWAY_RATE_LIMIT_ID) - const stub = ns.get(id) - const doRes = await stub.fetch(`http://localhost:8787/request`) - - const rateLimitResponse = await doRes.json() - t.falsy(rateLimitResponse.shouldBlock) -}) - -test('Receives should block when load reaches the RATE_LIMIT_REQUESTS', async (t) => { - const { mf } = t.context - const cid = 'bafkreidyeivj7adnnac6ljvzj2e3rd5xdw3revw4da7mx2ckrstapoupoq' - - const ns = await mf.getDurableObjectNamespace(getGatewayRateLimitsName()) - const id = ns.idFromName(GATEWAY_RATE_LIMIT_ID) - const stub = ns.get(id) - await Promise.all( - Array.from({ length: 100 }, (_, i) => - stub.fetch(`http://localhost:8787/request`) - ) - ) - - const doRes = await stub.fetch(`http://localhost:8787/request`) - - const shouldPreventRateLimit = await doRes.json() - t.truthy(shouldPreventRateLimit[gateways[0]]) - - const p = await mf.dispatchFetch(`https://${cid}.ipfs.localhost:8787`) - await p.waitUntil() - - const response = await mf.dispatchFetch('http://localhost:8787/metrics') - const metricsResponse = await response.text() - - t.is( - metricsResponse.includes( - `_prevented_requests_by_reason_total{gateway="${gateways[0]}",env="test",reason="RATE_LIMIT"} 1` - ), - true - ) -}) diff --git a/packages/edge-gateway/wrangler.toml b/packages/edge-gateway/wrangler.toml index ce8cda4..9decb25 100644 --- a/packages/edge-gateway/wrangler.toml +++ b/packages/edge-gateway/wrangler.toml @@ -21,7 +21,6 @@ bindings = [ {name = "GATEWAYMETRICS", class_name = "GatewayMetrics0"}, {name = "SUMMARYMETRICS", class_name = "SummaryMetrics0"}, {name = "CIDSTRACKER", class_name = "CidsTracker0"}, - {name = "GATEWAYRATELIMITS", class_name = "GatewayRateLimits4"}, {name = "GATEWAYREDIRECTCOUNTER", class_name = "GatewayRedirectCounter0"} ] @@ -34,7 +33,7 @@ route = "*.ipfs.nftstorage.link/*" kv_namespaces = [{ binding = "DENYLIST", id = "785cf627e913468ca5319523ae929def" }] [env.production.vars] -IPFS_GATEWAYS = "[\"https://ipfs.io\", \"https://cf.nftstorage.link\", \"https://nft-storage.mypinata.cloud/\"]" +IPFS_GATEWAYS = "[\"https://ipfs.io\", \"https://cf.nftstorage.link\", \"https://pinata.nftstorage.link\"]" GATEWAY_HOSTNAME = 'ipfs.nftstorage.link' DEBUG = "false" ENV = "production" @@ -44,7 +43,6 @@ bindings = [ {name = "GATEWAYMETRICS", class_name = "GatewayMetrics0"}, {name = "SUMMARYMETRICS", class_name = "SummaryMetrics0"}, {name = "CIDSTRACKER", class_name = "CidsTracker0"}, - {name = "GATEWAYRATELIMITS", class_name = "GatewayRateLimits4"}, {name = "GATEWAYREDIRECTCOUNTER", class_name = "GatewayRedirectCounter0"} ] @@ -57,7 +55,7 @@ route = "*.ipfs-staging.nftstorage.link/*" kv_namespaces = [{ binding = "DENYLIST", id = "f4eb0eca32e14e28b643604a82e00cb3" }] [env.staging.vars] -IPFS_GATEWAYS = "[\"https://ipfs.io\", \"https://cf.nftstorage.link\", \"https://nft-storage.mypinata.cloud/\"]" +IPFS_GATEWAYS = "[\"https://ipfs.io\", \"https://cf.nftstorage.link\", \"https://pinata.nftstorage.link\"]" GATEWAY_HOSTNAME = 'ipfs-staging.nftstorage.link' DEBUG = "true" ENV = "staging" @@ -67,7 +65,6 @@ bindings = [ {name = "GATEWAYMETRICS", class_name = "GatewayMetrics0"}, {name = "SUMMARYMETRICS", class_name = "SummaryMetrics0"}, {name = "CIDSTRACKER", class_name = "CidsTracker0"}, - {name = "GATEWAYRATELIMITS", class_name = "GatewayRateLimits4"}, {name = "GATEWAYREDIRECTCOUNTER", class_name = "GatewayRedirectCounter0"} ] @@ -87,7 +84,6 @@ bindings = [ {name = "GATEWAYMETRICS", class_name = "GatewayMetrics0"}, {name = "SUMMARYMETRICS", class_name = "SummaryMetrics0"}, {name = "CIDSTRACKER", class_name = "CidsTracker0"}, - {name = "GATEWAYRATELIMITS", class_name = "GatewayRateLimits4"}, {name = "GATEWAYREDIRECTCOUNTER", class_name = "GatewayRedirectCounter0"} ] @@ -106,7 +102,6 @@ bindings = [ {name = "GATEWAYMETRICS", class_name = "GatewayMetrics0"}, {name = "SUMMARYMETRICS", class_name = "SummaryMetrics0"}, {name = "CIDSTRACKER", class_name = "CidsTracker0"}, - {name = "GATEWAYRATELIMITS", class_name = "GatewayRateLimits4"}, {name = "GATEWAYREDIRECTCOUNTER", class_name = "GatewayRedirectCounter0"} ] @@ -124,7 +119,6 @@ bindings = [ {name = "GATEWAYMETRICS", class_name = "GatewayMetrics0"}, {name = "SUMMARYMETRICS", class_name = "SummaryMetrics0"}, {name = "CIDSTRACKER", class_name = "CidsTracker0"}, - {name = "GATEWAYRATELIMITS", class_name = "GatewayRateLimits4"}, {name = "GATEWAYREDIRECTCOUNTER", class_name = "GatewayRedirectCounter0"} ] @@ -148,4 +142,8 @@ deleted_classes = ["GatewayRateLimits2"] [[migrations]] tag = "v5" # Should be unique for each entry new_classes = ["GatewayRateLimits4"] -deleted_classes = ["GatewayRateLimits3"] \ No newline at end of file +deleted_classes = ["GatewayRateLimits3"] + +[[migrations]] +tag = "v5" # Should be unique for each entry +deleted_classes = ["GatewayRateLimits4"] \ No newline at end of file From 50af45c8f517be7f3912a2cdd1c677a61fb40d9b Mon Sep 17 00:00:00 2001 From: Vasco Santos Date: Mon, 25 Apr 2022 11:25:51 +0200 Subject: [PATCH 2/2] chore: apply suggestions from code review Co-authored-by: Oli Evans --- packages/edge-gateway/wrangler.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edge-gateway/wrangler.toml b/packages/edge-gateway/wrangler.toml index 9decb25..0a26bdf 100644 --- a/packages/edge-gateway/wrangler.toml +++ b/packages/edge-gateway/wrangler.toml @@ -145,5 +145,5 @@ new_classes = ["GatewayRateLimits4"] deleted_classes = ["GatewayRateLimits3"] [[migrations]] -tag = "v5" # Should be unique for each entry +tag = "v6" # Should be unique for each entry deleted_classes = ["GatewayRateLimits4"] \ No newline at end of file