Skip to content

Commit 3560c26

Browse files
committed
fix(analytics): validate against incomplete/invalid app_remove events
1 parent 5e2c5f8 commit 3560c26

File tree

2 files changed

+128
-5
lines changed

2 files changed

+128
-5
lines changed

spec/v1/providers/analytics.spec.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,92 @@ describe("Analytics Functions", () => {
301301
analyticsSpecInput.data
302302
);
303303
});
304+
305+
it("should handle null and missing user property values without throwing", () => {
306+
const cloudFunction = analytics
307+
.event("app_remove")
308+
.onLog((data: analytics.AnalyticsEvent) => data);
309+
310+
const event: Event = {
311+
data: {
312+
eventDim: [
313+
{
314+
name: "app_remove",
315+
params: {},
316+
date: "20240114",
317+
timestampMicros: "1705257600000000",
318+
},
319+
],
320+
userDim: {
321+
userProperties: {
322+
// Invalid properties that should be filtered out:
323+
null_property: null,
324+
value_null: {
325+
value: null,
326+
},
327+
value_undefined: {
328+
value: undefined,
329+
},
330+
empty_object: {},
331+
value_empty_object: {
332+
value: {},
333+
},
334+
// Valid properties that should be kept:
335+
valid_string: {
336+
value: {
337+
stringValue: "test",
338+
},
339+
setTimestampUsec: "1486076786090987",
340+
},
341+
valid_empty_string: {
342+
value: {
343+
stringValue: "",
344+
},
345+
setTimestampUsec: "1486076786090987",
346+
},
347+
valid_zero: {
348+
value: {
349+
intValue: "0",
350+
},
351+
setTimestampUsec: "1486076786090987",
352+
},
353+
},
354+
},
355+
},
356+
context: {
357+
eventId: "70172329041928",
358+
timestamp: "2018-04-09T07:56:12.975Z",
359+
eventType: "providers/google.firebase.analytics/eventTypes/event.log",
360+
resource: {
361+
service: "app-measurement.com",
362+
name: "projects/project1/events/app_remove",
363+
},
364+
},
365+
};
366+
367+
return expect(cloudFunction(event.data, event.context)).to.eventually.deep.equal({
368+
reportingDate: "20240114",
369+
name: "app_remove",
370+
params: {},
371+
logTime: "2024-01-14T18:40:00.000Z",
372+
user: {
373+
userProperties: {
374+
valid_string: {
375+
value: "test",
376+
setTime: "2017-02-02T23:06:26.090Z",
377+
},
378+
valid_empty_string: {
379+
value: "",
380+
setTime: "2017-02-02T23:06:26.090Z",
381+
},
382+
valid_zero: {
383+
value: "0",
384+
setTime: "2017-02-02T23:06:26.090Z",
385+
},
386+
},
387+
},
388+
});
389+
});
304390
});
305391
});
306392

src/v1/providers/analytics.ts

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,34 @@ export class UserDimensions {
179179
// The following fields do need transformations of some sort.
180180
copyTimestampToString(wireFormat, this, "firstOpenTimestampMicros", "firstOpenTime");
181181
this.userProperties = {}; // With no entries in the wire format, present an empty (as opposed to absent) map.
182-
copyField(wireFormat, this, "userProperties", (r) => {
183-
const entries = Object.entries(r).map(([k, v]) => [k, new UserPropertyValue(v)]);
182+
copyField(wireFormat, this, "userProperties", (r: unknown) => {
183+
const entries = Object.entries(r as Record<string, unknown>)
184+
.filter(([, v]) => {
185+
if (v === null || v === undefined) {
186+
return false;
187+
}
188+
if (typeof v !== "object") {
189+
return false;
190+
}
191+
const vObj = v as Record<string, unknown>;
192+
// Filter out empty objects
193+
if (Object.keys(vObj).length === 0) {
194+
return false;
195+
}
196+
// Filter if 'value' property exists but is null/undefined/empty object
197+
if ("value" in vObj) {
198+
const value = vObj.value;
199+
if (
200+
value === null ||
201+
value === undefined ||
202+
(typeof value === "object" && value !== null && Object.keys(value).length === 0)
203+
) {
204+
return false;
205+
}
206+
}
207+
return true;
208+
})
209+
.map(([k, v]) => [k, new UserPropertyValue(v)]);
184210
return Object.fromEntries(entries);
185211
});
186212
copyField(wireFormat, this, "bundleInfo", (r) => new ExportBundleInfo(r) as any);
@@ -454,9 +480,20 @@ function mapKeys<Obj, Transform extends (key: keyof Obj) => any>(
454480
// method always returns a string, similarly to avoid loss of precision, unlike the less-conservative
455481
// 'unwrapValue' method just below.
456482
/** @hidden */
457-
function unwrapValueAsString(wrapped: any): string {
458-
const key: string = Object.keys(wrapped)[0];
459-
return wrapped[key].toString();
483+
function unwrapValueAsString(wrapped: unknown): string {
484+
if (!wrapped || typeof wrapped !== "object") {
485+
return "";
486+
}
487+
const keys = Object.keys(wrapped);
488+
if (keys.length === 0) {
489+
return "";
490+
}
491+
const key: string = keys[0];
492+
const value = (wrapped as Record<string, unknown>)[key];
493+
if (value === null || value === undefined) {
494+
return "";
495+
}
496+
return value.toString();
460497
}
461498

462499
// Ditto as the method above, but returning the values in the idiomatic JavaScript type (string for strings,

0 commit comments

Comments
 (0)