diff --git a/services/web/app/src/infrastructure/GeoIpLookup.mjs b/services/web/app/src/infrastructure/GeoIpLookup.mjs index 110c0d2a00..8119217636 100644 --- a/services/web/app/src/infrastructure/GeoIpLookup.mjs +++ b/services/web/app/src/infrastructure/GeoIpLookup.mjs @@ -3,12 +3,26 @@ import settings from '@overleaf/settings' import logger from '@overleaf/logger' import { fetchJson } from '@overleaf/fetch-utils' +import LRU from 'lru-cache' /** * @typedef {import('../../../types/subscription/currency').CurrencyCode} CurrencyCode */ const DEFAULT_CURRENCY_CODE = /** @type {const} */ 'USD' +const cache = new LRU({ + max: settings.apis.geoIpLookup.cacheSize, +}) + +/** + * Cache details per /24 subnet, which is the smallest subnet routed on the public internet. + * IPv6 is not supported by GCP. We could cache by /48. + * @param {string} ip + */ +function networkCacheKey(ip) { + const octets = ip.split('.') + return octets.length === 4 ? octets.slice(0, 3).join('.') : ip +} /** @type {Record} */ const currencyMappings = { @@ -99,10 +113,17 @@ async function getDetails(ip, callback) { return } ip = ip.trim().split(' ')[0] + const cacheKey = networkCacheKey(ip) + const cached = cache.get(cacheKey) + if (cached) { + return cached + } const url = new URL(settings.apis.geoIpLookup.url) url.pathname += ip logger.debug({ ip, url }, 'getting geo ip details') - return await fetchJson(url, { signal: AbortSignal.timeout(1_000) }) + const details = await fetchJson(url, { signal: AbortSignal.timeout(1_000) }) + cache.set(cacheKey, details) + return details } /** diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index 5e80f0f42a..912e2cd0c8 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -232,6 +232,9 @@ module.exports = { '127.0.0.1' }:3003`, }, + geoIpLookup: { + cacheSize: intFromEnv('GEO_IP_LOOKUP_CACHE_SIZE', 10_000), + }, docstore: { url: `http://${process.env.DOCSTORE_HOST || '127.0.0.1'}:3016`, pubUrl: `http://${process.env.DOCSTORE_HOST || '127.0.0.1'}:3016`, diff --git a/services/web/test/unit/src/infrastructure/GeoIpLookup.test.mjs b/services/web/test/unit/src/infrastructure/GeoIpLookup.test.mjs index 858025be2d..a57a3f20b8 100644 --- a/services/web/test/unit/src/infrastructure/GeoIpLookup.test.mjs +++ b/services/web/test/unit/src/infrastructure/GeoIpLookup.test.mjs @@ -31,6 +31,7 @@ describe('GeoIpLookup', function () { apis: { geoIpLookup: { url: 'http://lookup.com/', + cacheSize: 10_000, }, }, } @@ -87,6 +88,20 @@ describe('GeoIpLookup', function () { new URL(ctx.settings.apis.geoIpLookup.url + ctx.ipAddress) ) }) + + it('should cache lookups by /24 network', async function (ctx) { + const first = await ctx.GeoIpLookup.promises.getDetails('12.34.56.78') + const second = await ctx.GeoIpLookup.promises.getDetails('12.34.56.99') + ctx.fetchUtils.fetchJson.should.have.been.calledOnce + assert.equal(first, second) + assert.deepEqual(second, ctx.stubbedResponse) + }) + + it('should not share cache entries across different /24 networks', async function (ctx) { + await ctx.GeoIpLookup.promises.getDetails('12.34.56.78') + await ctx.GeoIpLookup.promises.getDetails('12.34.99.1') + ctx.fetchUtils.fetchJson.should.have.been.calledTwice + }) }) })