Skip to content

Commit bd5962c

Browse files
committed
fix(openfeature): harden configuration source handling
1 parent b8020b2 commit bd5962c

5 files changed

Lines changed: 41 additions & 8 deletions

File tree

packages/dd-trace/src/openfeature/agentless_configuration_source.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class AgentlessConfigurationSource {
3232
this._clearTimeout = options.clearTimeout || clearTimeout
3333
this._started = false
3434
this._closed = false
35-
this._polling = false
35+
this._pollInFlight = false
3636
this._etag = undefined
3737
this._timer = undefined
3838
this._activeRequest = undefined
@@ -78,14 +78,14 @@ class AgentlessConfigurationSource {
7878
callback(null, { stopped: true })
7979
return
8080
}
81-
if (this._polling) {
81+
if (this._pollInFlight) {
8282
callback(null, { skipped: true })
8383
return
8484
}
8585

86-
this._polling = true
86+
this._pollInFlight = true
8787
this._attempt(1, (error, result) => {
88-
this._polling = false
88+
this._pollInFlight = false
8989
if (error && !this._closed) {
9090
log.debug('Feature Flagging agentless poll failed', error)
9191
}
@@ -273,7 +273,8 @@ class AgentlessConfigurationSource {
273273
*/
274274
function parseConfiguration (body) {
275275
const parsed = JSON.parse(body)
276-
if (!parsed || typeof parsed !== 'object' || !parsed.data || typeof parsed.data !== 'object') {
276+
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed) ||
277+
!parsed.data || typeof parsed.data !== 'object' || Array.isArray(parsed.data)) {
277278
throw new Error('Expected a JSON:API Universal Flag Configuration response')
278279
}
279280
if (parsed.data.type !== 'universal-flag-configuration') {
@@ -285,6 +286,7 @@ function parseConfiguration (body) {
285286
typeof configuration.createdAt !== 'string' ||
286287
(configuration.format !== undefined && typeof configuration.format !== 'string') ||
287288
!configuration.environment || typeof configuration.environment !== 'object' ||
289+
Array.isArray(configuration.environment) ||
288290
typeof configuration.environment.name !== 'string' ||
289291
!configuration.flags || typeof configuration.flags !== 'object' || Array.isArray(configuration.flags)) {
290292
const keys = configuration && typeof configuration === 'object'

packages/dd-trace/src/openfeature/configuration_source.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ function resolve (config) {
2626
return {
2727
mode,
2828
endpoint: endpoint(config, config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_BASE_URL),
29-
pollIntervalMs: positiveMilliseconds(
29+
pollIntervalMs: positiveMillisecondsFromSeconds(
3030
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_POLL_INTERVAL_SECONDS,
3131
DEFAULT_POLL_INTERVAL_SECONDS,
3232
'poll interval',
3333
MAX_POLL_INTERVAL_SECONDS
3434
),
35-
requestTimeoutMs: positiveMilliseconds(
35+
requestTimeoutMs: positiveMillisecondsFromSeconds(
3636
config.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE_AGENTLESS_REQUEST_TIMEOUT_SECONDS,
3737
DEFAULT_REQUEST_TIMEOUT_SECONDS,
3838
'request timeout'
@@ -150,7 +150,7 @@ function endpoint (config, configuredBaseUrl) {
150150
* @param {number} [maximumSeconds] - Optional inclusive maximum.
151151
* @returns {number} Positive milliseconds.
152152
*/
153-
function positiveMilliseconds (value, fallbackSeconds, setting, maximumSeconds) {
153+
function positiveMillisecondsFromSeconds (value, fallbackSeconds, setting, maximumSeconds) {
154154
const seconds = Number(value)
155155
if (!Number.isFinite(seconds) || seconds <= 0) {
156156
log.warn(

packages/dd-trace/src/openfeature/flagging_provider.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ class FlaggingProvider extends DatadogNodeServerProvider {
6767
*/
6868
_setConfigurationSource (source) {
6969
if (this.#configurationSource) {
70+
log.warn('%s already has a configuration source; ignoring duplicate source', this.constructor.name)
7071
source.stop()
7172
return
7273
}

packages/dd-trace/test/openfeature/agentless_configuration_source.spec.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,31 @@ describe('AgentlessConfigurationSource', () => {
301301
sinon.assert.calledThrice(log.debug)
302302
})
303303

304+
it('rejects arrays where UFC envelope objects are required', async () => {
305+
const configuration = JSON.parse(VALID_UFC)
306+
responses.push(
307+
{ statusCode: 200, body: JSON.stringify([]) },
308+
{ statusCode: 200, body: JSON.stringify({ data: [] }) },
309+
{
310+
statusCode: 200,
311+
body: JSON.stringify({
312+
data: {
313+
type: 'universal-flag-configuration',
314+
attributes: { ...configuration, environment: [] },
315+
},
316+
}),
317+
}
318+
)
319+
const configurationSource = source()
320+
321+
await poll(configurationSource)
322+
await poll(configurationSource)
323+
await poll(configurationSource)
324+
325+
sinon.assert.notCalled(applyConfiguration)
326+
sinon.assert.calledThrice(log.debug)
327+
})
328+
304329
it('preserves last-known-good configuration and ETag after malformed JSON', async () => {
305330
responses.push(
306331
{ statusCode: 200, headers: { etag: '"good"' }, body: VALID_RESPONSE },

packages/dd-trace/test/openfeature/flagging_provider.spec.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,11 @@ describe('FlaggingProvider', () => {
224224
sinon.assert.notCalled(first.stop)
225225
sinon.assert.notCalled(duplicate.start)
226226
sinon.assert.calledOnce(duplicate.stop)
227+
sinon.assert.calledOnceWithExactly(
228+
log.warn,
229+
'%s already has a configuration source; ignoring duplicate source',
230+
'FlaggingProvider'
231+
)
227232
})
228233
})
229234

0 commit comments

Comments
 (0)