diff --git a/superset-frontend/src/middleware/asyncEvent.test.ts b/superset-frontend/src/middleware/asyncEvent.test.ts index 10da96099dd9..0ec0031514ec 100644 --- a/superset-frontend/src/middleware/asyncEvent.test.ts +++ b/superset-frontend/src/middleware/asyncEvent.test.ts @@ -172,6 +172,34 @@ describe('asyncEvent middleware', () => { expect(fetchMock.callHistory.calls(EVENTS_ENDPOINT)).toHaveLength(1); expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength(1); }); + + // Regression guard for the motivating CodeQL case: a job_id that collides + // with a built-in Object property (e.g. "__proto__"/"constructor") must be + // routed through the Map-based registries without triggering prototype + // pollution or losing the listener to a prototype-bearing lookup. + test.each(['__proto__', 'constructor', 'prototype', 'hasOwnProperty'])( + 'resolves listeners keyed by reserved job_id "%s"', + async jobId => { + fetchMock.clearHistory().removeRoutes(); + fetchMock.get(EVENTS_ENDPOINT, { + status: 200, + body: { result: [{ ...asyncDoneEvent, job_id: jobId }] }, + }); + fetchMock.get(CACHED_DATA_ENDPOINT, { + status: 200, + body: { result: chartData }, + }); + + const actualResolved = await asyncEvent.waitForAsyncData({ + ...asyncPendingEvent, + job_id: jobId, + }); + expect(actualResolved).toEqual([chartData]); + expect(fetchMock.callHistory.calls(CACHED_DATA_ENDPOINT)).toHaveLength( + 1, + ); + }, + ); }); // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks diff --git a/superset-frontend/src/middleware/asyncEvent.ts b/superset-frontend/src/middleware/asyncEvent.ts index 4fe60d2a4a96..74e8c055fce1 100644 --- a/superset-frontend/src/middleware/asyncEvent.ts +++ b/superset-frontend/src/middleware/asyncEvent.ts @@ -63,17 +63,17 @@ let config: AppConfig; let transport: string; let pollingDelayMs: number; let pollingTimeoutId: number; -let listenersByJobId: Record; -let retriesByJobId: Record; +let listenersByJobId: Map; +let retriesByJobId: Map; let lastReceivedEventId: string | null | undefined; -const addListener = (id: string, fn: any) => { - listenersByJobId[id] = fn; +const addListener = (id: string, fn: ListenerFn) => { + listenersByJobId.set(id, fn); }; const removeListener = (id: string) => { - if (!listenersByJobId[id]) return; - delete listenersByJobId[id]; + if (!listenersByJobId.has(id)) return; + listenersByJobId.delete(id); }; const fetchCachedData = async ( @@ -143,27 +143,22 @@ const setLastId = (asyncEvent: AsyncEvent) => { export const processEvents = async (events: AsyncEvent[]) => { events.forEach((asyncEvent: AsyncEvent) => { const jobId = asyncEvent.job_id; - const listener = Object.prototype.hasOwnProperty.call( - listenersByJobId, - jobId, - ) - ? listenersByJobId[jobId] - : undefined; + const listener = listenersByJobId.get(jobId); if (listener) { listener(asyncEvent); - delete retriesByJobId[jobId]; + retriesByJobId.delete(jobId); } else { // handle race condition where event is received // before listener is registered - if (!retriesByJobId[jobId]) retriesByJobId[jobId] = 0; - retriesByJobId[jobId] += 1; + const retries = (retriesByJobId.get(jobId) ?? 0) + 1; + retriesByJobId.set(jobId, retries); - if (retriesByJobId[jobId] <= MAX_RETRIES) { + if (retries <= MAX_RETRIES) { setTimeout(() => { processEvents([asyncEvent]); - }, RETRY_DELAY * retriesByJobId[jobId]); + }, RETRY_DELAY * retries); } else { - delete retriesByJobId[jobId]; + retriesByJobId.delete(jobId); logging.warn('listener not found for job_id', asyncEvent.job_id); } } @@ -173,7 +168,7 @@ export const processEvents = async (events: AsyncEvent[]) => { const loadEventsFromApi = async () => { const eventArgs = lastReceivedEventId ? { last_id: lastReceivedEventId } : {}; - if (Object.keys(listenersByJobId).length) { + if (listenersByJobId.size) { try { const { result: events } = await fetchEvents(eventArgs); if (events?.length) await processEvents(events); @@ -236,8 +231,8 @@ export const init = (appConfig?: AppConfig) => { if (!isFeatureEnabled(FeatureFlag.GlobalAsyncQueries)) return; if (pollingTimeoutId) clearTimeout(pollingTimeoutId); - listenersByJobId = {}; - retriesByJobId = {}; + listenersByJobId = new Map(); + retriesByJobId = new Map(); lastReceivedEventId = null; config = appConfig || getBootstrapData().common.conf;