diff --git a/packages/shared/common/__tests__/internal/fdv2/protocolHandler.test.ts b/packages/shared/common/__tests__/internal/fdv2/protocolHandler.test.ts index 1b3d099e80..940404f1c3 100644 --- a/packages/shared/common/__tests__/internal/fdv2/protocolHandler.test.ts +++ b/packages/shared/common/__tests__/internal/fdv2/protocolHandler.test.ts @@ -202,14 +202,67 @@ it('ignores put-objects received before a server-intent', () => { expect(action.type).toBe('none'); }); -it('silently ignores put-objects with unrecognized kinds', () => { - const handler = createHandler(); +it('silently ignores put-objects and delete-objects with unsupported kinds', () => { + const logger: LDLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; + const handler = createProtocolHandler(passthrough, logger); + handler.processEvent(intentEvent('xfer-changes')); + + // Interleave objects of an unsupported kind with supported ones. + const actions = [ + putEvent('flag', 'f1', 1, { on: true }), + putEvent('some-future-kind', 'x1', 1, { foo: 1 }), + deleteEvent('some-future-kind', 'x2', 2), + deleteEvent('segment', 's1', 3), + transferredEvent('s', 1), + ].map((event) => handler.processEvent(event)); + + // No event is rejected, the payload holds only the supported objects, and nothing is + // logged above debug. + const action = actions.pop()!; + expect(actions.every((other) => other.type === 'none')).toBe(true); + expect(action.type).toBe('payload'); + if (action.type !== 'payload') return; + expect(action.payload.updates).toEqual([ + { kind: 'flag', key: 'f1', version: 1, object: { on: true } }, + { kind: 'segment', key: 's1', version: 3, deleted: true }, + ]); + expect(logger.warn).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); +}); + +it.each(['constructor', 'toString', '__proto__'])( + 'ignores objects whose kind names an inherited object member: %s', + (kind) => { + const handler = createHandler(); + handler.processEvent(intentEvent('xfer-changes')); + handler.processEvent(putEvent('flag', 'f1', 1, { on: true })); + + // Send a put and a delete whose kind is not a registered processor. + handler.processEvent(putEvent(kind, 'x1', 1, { foo: 1 })); + handler.processEvent(deleteEvent(kind, 'x2', 2)); + const action = handler.processEvent(transferredEvent('s', 1)); + + // Only the supported object is in the payload. + expect(action.type).toBe('payload'); + if (action.type !== 'payload') return; + expect(action.payload.updates.map((update) => update.key)).toEqual(['f1']); + }, +); + +it('warns when the processor for a supported kind rejects the object', () => { + const logger: LDLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; + const handler = createProtocolHandler({ flag: () => undefined }, logger); handler.processEvent(intentEvent('xfer-full')); - handler.processEvent(putEvent('unknownKind', 'f1', 1, { on: true })); + // The processor for a supported kind rejects the object. + handler.processEvent(putEvent('flag', 'f1', 1, { on: true })); + + // The object is dropped and the rejection is reported. const action = handler.processEvent(transferredEvent('s', 1)); + expect(action.type).toBe('payload'); if (action.type !== 'payload') return; expect(action.payload.updates).toHaveLength(0); + expect(logger.warn).toHaveBeenCalledTimes(1); }); it('accumulates delete-objects during a transfer', () => { diff --git a/packages/shared/common/src/internal/fdv2/protocolHandler.ts b/packages/shared/common/src/internal/fdv2/protocolHandler.ts index c5e12669a0..373dbdacfb 100644 --- a/packages/shared/common/src/internal/fdv2/protocolHandler.ts +++ b/packages/shared/common/src/internal/fdv2/protocolHandler.ts @@ -88,6 +88,12 @@ export function createProtocolHandler( let tempType: PayloadType = 'partial'; let tempUpdates: Update[] = []; + // Only kinds with a registered processor are supported. An own-property check keeps + // kinds such as 'constructor' from resolving to inherited members. + function isSupportedKind(kind: string): boolean { + return Object.prototype.hasOwnProperty.call(objProcessors, kind); + } + function processObj(kind: string, jsonObj: any): any { return objProcessors[kind]?.(jsonObj); } @@ -171,6 +177,13 @@ export function createProtocolHandler( return ACTION_NONE; } + // Objects of unsupported kinds are dropped silently so the protocol can add new kinds + // without disrupting older SDKs. + if (!isSupportedKind(data.kind)) { + logger?.debug(`Ignoring put-object with unsupported kind: '${data.kind}'`); + return ACTION_NONE; + } + const obj = processObj(data.kind, data.object); if (!obj) { logger?.warn(`Unable to process object for kind: '${data.kind}'`); @@ -199,6 +212,11 @@ export function createProtocolHandler( return ACTION_NONE; } + if (!isSupportedKind(data.kind)) { + logger?.debug(`Ignoring delete-object with unsupported kind: '${data.kind}'`); + return ACTION_NONE; + } + tempUpdates.push({ kind: data.kind, key: data.key, diff --git a/packages/shared/sdk-client/__tests__/datasource/fdv2/StreamingFDv2Base.test.ts b/packages/shared/sdk-client/__tests__/datasource/fdv2/StreamingFDv2Base.test.ts index 723b9364d3..5a56b8f7dd 100644 --- a/packages/shared/sdk-client/__tests__/datasource/fdv2/StreamingFDv2Base.test.ts +++ b/packages/shared/sdk-client/__tests__/datasource/fdv2/StreamingFDv2Base.test.ts @@ -204,6 +204,11 @@ it('silently ignores unrecognized object kinds', async () => { version: 1, object: { data: 'ignored' }, }); + simulateEvent(mockEventSource, 'delete-object', { + kind: 'unknown_kind', + key: 'mystery-deleted', + version: 2, + }); simulateEvent(mockEventSource, 'payload-transferred', { state: '(p:p1:1)', version: 1 }); const result = await base.takeResult(); @@ -212,6 +217,8 @@ it('silently ignores unrecognized object kinds', async () => { expect(result.payload.updates).toHaveLength(1); expect(result.payload.updates[0].key).toBe('known'); + expect(logger.warn).not.toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); base.close(); }); diff --git a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts index 688a11f89d..f5cd65d46e 100644 --- a/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts +++ b/packages/shared/sdk-server/__tests__/data_sources/PollingProcessorFDv2.test.ts @@ -101,6 +101,58 @@ describe('given a polling processor', () => { }); }); + it('applies supported objects and ignores objects of unsupported kinds', () => { + const logger = new TestLogger(); + processor = new PollingProcessorFDv2(requestor as unknown as Requestor, longInterval, logger); + const body = JSON.stringify({ + events: [ + { + event: 'server-intent', + data: { payloads: [{ intentCode: 'xfer-full', id: 'mockId' }] }, + }, + { + event: 'put-object', + data: { kind: 'flag', key: 'flagA', version: 1, object: { key: 'flagA', version: 1 } }, + }, + { + event: 'put-object', + data: { kind: 'some-future-kind', key: 'futureA', version: 1, object: { foo: 1 } }, + }, + { + event: 'delete-object', + data: { kind: 'some-future-kind', key: 'futureB', version: 2 }, + }, + { + event: 'put-object', + data: { kind: 'segment', key: 'segA', version: 1, object: { key: 'segA', version: 1 } }, + }, + { + event: 'payload-transferred', + data: { state: 'mockState', version: 1 }, + }, + ], + }); + requestor.requestAllData = jest.fn((cb) => cb(undefined, body, headers)); + const dataCallback = jest.fn(); + const statusCallback = jest.fn(); + + // Poll a response that mixes supported and unsupported kinds. + processor.start(dataCallback, statusCallback); + + // Only the supported objects are delivered, and the source reports valid without warnings. + expect(dataCallback).toHaveBeenCalledTimes(1); + expect(dataCallback.mock.calls[0][1].payload.updates).toEqual([ + { kind: 'flag', key: 'flagA', version: 1, object: { key: 'flagA', version: 1 } }, + { kind: 'segment', key: 'segA', version: 1, object: { key: 'segA', version: 1 } }, + ]); + expect(statusCallback.mock.calls).toEqual([ + [subsystem.DataSourceState.Initializing], + [subsystem.DataSourceState.Valid], + ]); + expect(logger.getCount(LogLevel.Warn)).toBe(0); + expect(logger.getCount(LogLevel.Error)).toBe(0); + }); + it('can process FDv1 data when configured to do so', async () => { processor = new PollingProcessorFDv2( requestor as unknown as Requestor,