fix: put [5s, 1d] range guard on the delay before re-fetching central config (#185)

Refs: elastic/apm-agent-nodejs#2941

Author: trentm

CHANGELOG.md

@@ -1,5 +1,12 @@
 # elastic-apm-http-client changelog
 
+## Unreleased
+
+- Add guards to ensure that a crazy `Cache-Control: max-age=...` response
+  header cannot accidentally result in inappropriate intervals for fetching
+  central config. The re-fetch delay is clamped to `[5 seconds, 1 day]`.
+  (https://github.com/elastic/apm-agent-nodejs/issues/2941)
+
 ## v11.0.1
 
 - Fix an issue when running in a Lambda function, where a missing or erroring

index.js

@@ -25,6 +25,7 @@ const StreamChopper = require('stream-chopper')
 const ndjson = require('./lib/ndjson')
 const { NoopLogger } = require('./lib/logging')
 const truncate = require('./lib/truncate')
+const { getCentralConfigIntervalS } = require('./lib/central-config')
 
 module.exports = Client
 
@@ -432,12 +433,11 @@ Client.prototype._pollConfig = function () {
 Client.prototype._scheduleNextConfigPoll = function (seconds) {
   if (this._configTimer !== null) return
 
-  seconds = seconds || 300
-
+  const delayS = getCentralConfigIntervalS(seconds)
   this._configTimer = setTimeout(() => {
     this._configTimer = null
     this._pollConfig()
-  }, seconds * 1000)
+  }, delayS * 1000)
 
   this._configTimer.unref()
 }
@@ -1464,9 +1464,10 @@ function normalizeGlobalLabels (labels) {
   return result
 }
 
+// https://httpwg.org/specs/rfc9111.html#cache-response-directive.max-age
 function getMaxAge (res) {
   const header = res.headers['cache-control']
-  const match = header && header.match(/max-age=(\d+)/)
+  const match = header && header.match(/max-age=(\d+)/i)
   return parseInt(match && match[1], 10)
 }
 

lib/central-config.js

@@ -0,0 +1,35 @@
+'use strict'
+
+// Central config-related utilities for the APM http client.
+
+const INTERVAL_DEFAULT_S = 300 // 5 min
+const INTERVAL_MIN_S = 5
+const INTERVAL_MAX_S = 86400 // 1d
+
+/**
+ * Determine an appropriate delay until the next fetch of Central Config.
+ * Default to 5 minutes, minimum 5s, max 1d.
+ *
+ * The maximum of 1d ensures we don't get surprised by an overflow value to
+ * `setTimeout` per https://developer.mozilla.org/en-US/docs/Web/API/setTimeout#maximum_delay_value
+ *
+ * @param {Number|undefined} seconds - A number of seconds, typically pulled
+ *    from a `Cache-Control: max-age=${seconds}` header on a previous central
+ *    config request.
+ * @returns {Number}
+ */
+function getCentralConfigIntervalS (seconds) {
+  if (typeof seconds !== 'number' || seconds <= 0) {
+    return INTERVAL_DEFAULT_S
+  }
+  return Math.min(Math.max(seconds, INTERVAL_MIN_S), INTERVAL_MAX_S)
+}
+
+module.exports = {
+  getCentralConfigIntervalS,
+
+  // These are exported for testing.
+  INTERVAL_DEFAULT_S,
+  INTERVAL_MIN_S,
+  INTERVAL_MAX_S
+}

test/central-config.test.js

@@ -3,8 +3,53 @@
 const test = require('tape')
 
 const { APMServer, validOpts, assertConfigReq } = require('./lib/utils')
+const {
+  getCentralConfigIntervalS,
+  INTERVAL_DEFAULT_S,
+  INTERVAL_MIN_S,
+  INTERVAL_MAX_S
+} = require('../lib/central-config')
 const Client = require('../')
 
+test('getCentralConfigIntervalS', function (t) {
+  const testCases = [
+    // [ <input arg>, <expected result> ]
+    [-4, INTERVAL_DEFAULT_S],
+    [-1, INTERVAL_DEFAULT_S],
+    [0, 300],
+    [1, INTERVAL_MIN_S],
+    [2, INTERVAL_MIN_S],
+    [3, INTERVAL_MIN_S],
+    [4, INTERVAL_MIN_S],
+    [5, INTERVAL_MIN_S],
+    [6, 6],
+    [7, 7],
+    [8, 8],
+    [9, 9],
+    [10, 10],
+    [86398, 86398],
+    [86399, 86399],
+    [86400, 86400],
+    [86401, INTERVAL_MAX_S],
+    [86402, INTERVAL_MAX_S],
+    [86403, INTERVAL_MAX_S],
+    [86404, INTERVAL_MAX_S],
+    [null, INTERVAL_DEFAULT_S],
+    [undefined, INTERVAL_DEFAULT_S],
+    [false, INTERVAL_DEFAULT_S],
+    [true, INTERVAL_DEFAULT_S],
+    ['a string', INTERVAL_DEFAULT_S],
+    [{}, INTERVAL_DEFAULT_S],
+    [[], INTERVAL_DEFAULT_S]
+  ]
+
+  testCases.forEach(testCase => {
+    t.equal(getCentralConfigIntervalS(testCase[0]), testCase[1],
+      `getCentralConfigIntervalS(${testCase[0]}) -> ${testCase[1]}`)
+  })
+  t.end()
+})
+
 test('central config disabled', function (t) {
   const origPollConfig = Client.prototype._pollConfig
   Client.prototype._pollConfig = function () {