Skip to content

Commit d36d124

Browse files
ibesoragithub-actions[bot]
ibesora
authored and
github-actions[bot]
committed
Fix wrong access token used on multi-map environments
GitOrigin-RevId: 5a7c5e13f3ad06d242c52ce748fcfdebbfc23009
1 parent b31c892 commit d36d124

2 files changed

Lines changed: 51 additions & 43 deletions

File tree

‎src/util/mapbox.ts‎

Lines changed: 31 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -355,13 +355,20 @@ export function setBundleDistribution(distribution: 'cdn' | 'other') {
355355

356356
type TelemetryEventType = 'appUserTurnstile' | 'map.load' | 'map.auth' | 'gljs.performance' | 'style.load' | 'metrics';
357357

358+
type QueueItem = {
359+
id?: number;
360+
timestamp: number;
361+
performanceData?: LivePerformanceData;
362+
payload?: StyleLoadEventPayload | MetricsEventPayload;
363+
customAccessToken?: string | null;
364+
};
365+
358366
export class TelemetryEvent {
359367
// eslint-disable-next-line @typescript-eslint/no-explicit-any
360368
eventData: any;
361369
anonId: string | null | undefined;
362370
anonIdTimestamp: number | null | undefined;
363-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
364-
queue: Array<any>;
371+
queue: Array<QueueItem>;
365372
type: TelemetryEventType;
366373
pendingRequest: boolean;
367374
_customAccessToken: string | null | undefined;
@@ -454,7 +461,7 @@ export class TelemetryEvent {
454461

455462
}
456463

457-
processRequests(_?: string | null) {}
464+
processRequests() {}
458465

459466
/*
460467
* If any event data should be persisted after the POST request, the callback should modify eventData`
@@ -486,19 +493,19 @@ export class TelemetryEvent {
486493
this.pendingRequest = false;
487494
callback(null);
488495
this.saveEventData();
489-
this.processRequests(customAccessToken);
496+
this.processRequests();
490497
})
491498
.catch((err: Error) => {
492499
this.pendingRequest = false;
493500
callback(err);
494501
this.saveEventData();
495-
this.processRequests(customAccessToken);
502+
this.processRequests();
496503
});
497504
}
498505

499-
queueRequest(event: unknown, customAccessToken?: string | null) {
500-
this.queue.push(event);
501-
this.processRequests(customAccessToken);
506+
queueRequest(event: Omit<QueueItem, 'customAccessToken'>, customAccessToken?: string | null) {
507+
this.queue.push({...event, customAccessToken});
508+
this.processRequests();
502509
}
503510
}
504511

@@ -512,15 +519,13 @@ export class PerformanceEvent extends TelemetryEvent {
512519
this.queueRequest({timestamp: Date.now(), performanceData}, customAccessToken);
513520
}
514521

515-
override processRequests(customAccessToken?: string | null) {
522+
override processRequests() {
516523
if (this.pendingRequest || this.queue.length === 0) {
517524
return;
518525
}
519526

520-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
521-
const {timestamp, performanceData} = this.queue.shift();
527+
const {timestamp, performanceData, customAccessToken} = this.queue.shift();
522528

523-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
524529
const additionalPayload = getLivePerformanceMetrics(performanceData);
525530

526531
// Server will only process string for these entries
@@ -534,7 +539,6 @@ export class PerformanceEvent extends TelemetryEvent {
534539
assert(typeof attribute.value === 'string');
535540
}
536541

537-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
538542
this.postEvent(timestamp, additionalPayload, () => {}, customAccessToken);
539543
}
540544
}
@@ -565,13 +569,11 @@ export class MapLoadEvent extends TelemetryEvent {
565569
}
566570
}
567571

568-
override processRequests(customAccessToken?: string | null) {
572+
override processRequests() {
569573
if (this.pendingRequest || this.queue.length === 0) return;
570-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
571-
const {id, timestamp} = this.queue.shift();
574+
const {id, timestamp, customAccessToken} = this.queue.shift();
572575

573576
// Only one load event should fire per map
574-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
575577
if (id && this.success[id]) return;
576578

577579
if (!this.anonId || !this.anonIdTimestamp) {
@@ -595,15 +597,12 @@ export class MapLoadEvent extends TelemetryEvent {
595597

596598
if (sdkInfo) additionalPayload.sdkInfo = sdkInfo;
597599

598-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
599600
this.postEvent(timestamp, additionalPayload, (err) => {
600601
if (err) {
601602
this.errorCb(err);
602603
} else {
603-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
604604
if (id) this.success[id] = true;
605605
}
606-
607606
}, customAccessToken);
608607
}
609608

@@ -678,15 +677,13 @@ export class StyleLoadEvent extends TelemetryEvent {
678677
}, customAccessToken);
679678
}
680679

681-
override processRequests(customAccessToken?: string | null) {
680+
override processRequests() {
682681
if (this.pendingRequest || this.queue.length === 0) {
683682
return;
684683
}
685684

686-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
687-
const {timestamp, payload} = this.queue.shift();
685+
const {timestamp, payload, customAccessToken} = this.queue.shift();
688686

689-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
690687
this.postEvent(timestamp, payload, () => {}, customAccessToken);
691688
}
692689
}
@@ -731,16 +728,14 @@ class MetricsEvent extends TelemetryEvent {
731728
}, customAccessToken);
732729
}
733730

734-
override processRequests(customAccessToken?: string | null) {
731+
override processRequests() {
735732
// Override processRequests to allow multiple events sent per map
736733
if (this.pendingRequest || this.queue.length === 0) {
737734
return;
738735
}
739736

740-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
741-
const {timestamp, payload} = this.queue.shift();
737+
const {timestamp, payload, customAccessToken} = this.queue.shift();
742738

743-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
744739
this.postEvent(timestamp, payload, () => {}, customAccessToken);
745740
}
746741
}
@@ -777,13 +772,13 @@ export class MapSessionAPI extends TelemetryEvent {
777772
this.pendingRequest = false;
778773
callback(null);
779774
this.saveEventData();
780-
this.processRequests(customAccessToken);
775+
this.processRequests();
781776
})
782777
.catch((err: Error) => {
783778
this.pendingRequest = false;
784779
callback(err);
785780
this.saveEventData();
786-
this.processRequests(customAccessToken);
781+
this.processRequests();
787782
});
788783
}
789784

@@ -800,21 +795,17 @@ export class MapSessionAPI extends TelemetryEvent {
800795
}
801796
}
802797

803-
override processRequests(customAccessToken?: string | null) {
798+
override processRequests() {
804799
if (this.pendingRequest || this.queue.length === 0) return;
805-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
806-
const {id, timestamp} = this.queue.shift();
800+
const {id, timestamp, customAccessToken} = this.queue.shift();
807801

808802
// Only one load event should fire per map
809-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
810803
if (id && this.success[id]) return;
811804

812-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
813805
this.getSession(timestamp, this.skuToken, (err) => {
814806
if (err) {
815807
this.errorCb(err);
816808
} else {
817-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
818809
if (id) this.success[id] = true;
819810
}
820811
}, customAccessToken);
@@ -837,11 +828,11 @@ export class TurnstileEvent extends TelemetryEvent {
837828
// mapbox tiles.
838829
if (Array.isArray(tileUrls) &&
839830
tileUrls.some(url => isMapboxURL(url) || isMapboxHTTPURL(url))) {
840-
this.queueRequest(Date.now(), customAccessToken);
831+
this.queueRequest({timestamp: Date.now()}, customAccessToken);
841832
}
842833
}
843834

844-
override processRequests(customAccessToken?: string | null) {
835+
override processRequests() {
845836
if (this.pendingRequest || this.queue.length === 0) {
846837
return;
847838
}
@@ -863,14 +854,12 @@ export class TurnstileEvent extends TelemetryEvent {
863854
dueForEvent = true;
864855
}
865856

866-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
867-
const nextUpdate = this.queue.shift();
857+
const {timestamp: nextUpdate, customAccessToken} = this.queue.shift();
868858
// Record turnstile event once per calendar day.
869859
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
870860
if (this.eventData.lastSuccess) {
871861
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-member-access
872862
const lastUpdate = new Date(this.eventData.lastSuccess);
873-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
874863
const nextDate = new Date(nextUpdate);
875864
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
876865
const daysElapsed = (nextUpdate - this.eventData.lastSuccess) / (24 * 60 * 60 * 1000);
@@ -897,10 +886,9 @@ export class TurnstileEvent extends TelemetryEvent {
897886

898887
if (sdkInfo) additionalPayload.sdkInfo = sdkInfo;
899888

900-
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
901889
this.postEvent(nextUpdate, additionalPayload, (err) => {
902890
if (!err) {
903-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
891+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
904892
this.eventData.lastSuccess = nextUpdate;
905893
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
906894
this.eventData.tokenU = tokenU;

‎test/unit/util/mapbox.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,26 @@ describe("mapbox", () => {
12611261
expect(mapLoadEvent.bundleDistribution).toEqual('other');
12621262
});
12631263

1264+
test('drains the second queued event using its own access token, not the first event\'s stale token', async () => {
1265+
// Simulates two Map instances with different per-Map accessTokens posting
1266+
// telemetry through the shared MapLoadEvent singleton while a request is in flight.
1267+
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
1268+
event.postMapLoadEvent(1, skuToken, 'token-for-map-a', () => {});
1269+
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access
1270+
event.postMapLoadEvent(2, skuToken, 'token-for-map-b', () => {});
1271+
1272+
// Let the first request's promise chain settle, which triggers draining the
1273+
// second queued event.
1274+
await new Promise(resolve => { setTimeout(resolve, 0); });
1275+
1276+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
1277+
expect(window.server.requests.length).toEqual(2);
1278+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
1279+
expect(window.server.requests[0].url).toContain('access_token=token-for-map-a');
1280+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
1281+
expect(window.server.requests[1].url).toContain('access_token=token-for-map-b');
1282+
});
1283+
12641284
test('setSdkInfo ignores invalid values', async () => {
12651285
mapbox.setSdkInfo('not a valid value!');
12661286
// eslint-disable-next-line @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-member-access

0 commit comments

Comments
 (0)