From 08a964ebeb0c00ffd9256efcbf278d2603c43ead Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Tue, 30 Sep 2025 12:34:20 +0100 Subject: [PATCH 1/7] fix(analytics): validate against incomplete/invalid app_remove events --- spec/v1/providers/analytics.spec.ts | 86 +++++++++++++++++++++++++++++ src/v1/providers/analytics.ts | 47 ++++++++++++++-- 2 files changed, 128 insertions(+), 5 deletions(-) diff --git a/spec/v1/providers/analytics.spec.ts b/spec/v1/providers/analytics.spec.ts index 98db1702f..5b5b0c281 100644 --- a/spec/v1/providers/analytics.spec.ts +++ b/spec/v1/providers/analytics.spec.ts @@ -301,6 +301,92 @@ describe("Analytics Functions", () => { analyticsSpecInput.data ); }); + + it("should handle null and missing user property values without throwing", () => { + const cloudFunction = analytics + .event("app_remove") + .onLog((data: analytics.AnalyticsEvent) => data); + + const event: Event = { + data: { + eventDim: [ + { + name: "app_remove", + params: {}, + date: "20240114", + timestampMicros: "1705257600000000", + }, + ], + userDim: { + userProperties: { + // Invalid properties that should be filtered out: + null_property: null, + value_null: { + value: null, + }, + value_undefined: { + value: undefined, + }, + empty_object: {}, + value_empty_object: { + value: {}, + }, + // Valid properties that should be kept: + valid_string: { + value: { + stringValue: "test", + }, + setTimestampUsec: "1486076786090987", + }, + valid_empty_string: { + value: { + stringValue: "", + }, + setTimestampUsec: "1486076786090987", + }, + valid_zero: { + value: { + intValue: "0", + }, + setTimestampUsec: "1486076786090987", + }, + }, + }, + }, + context: { + eventId: "70172329041928", + timestamp: "2018-04-09T07:56:12.975Z", + eventType: "providers/google.firebase.analytics/eventTypes/event.log", + resource: { + service: "app-measurement.com", + name: "projects/project1/events/app_remove", + }, + }, + }; + + return expect(cloudFunction(event.data, event.context)).to.eventually.deep.equal({ + reportingDate: "20240114", + name: "app_remove", + params: {}, + logTime: "2024-01-14T18:40:00.000Z", + user: { + userProperties: { + valid_string: { + value: "test", + setTime: "2017-02-02T23:06:26.090Z", + }, + valid_empty_string: { + value: "", + setTime: "2017-02-02T23:06:26.090Z", + }, + valid_zero: { + value: "0", + setTime: "2017-02-02T23:06:26.090Z", + }, + }, + }, + }); + }); }); }); diff --git a/src/v1/providers/analytics.ts b/src/v1/providers/analytics.ts index 63895a7ca..9136ab293 100644 --- a/src/v1/providers/analytics.ts +++ b/src/v1/providers/analytics.ts @@ -179,8 +179,34 @@ export class UserDimensions { // The following fields do need transformations of some sort. copyTimestampToString(wireFormat, this, "firstOpenTimestampMicros", "firstOpenTime"); this.userProperties = {}; // With no entries in the wire format, present an empty (as opposed to absent) map. - copyField(wireFormat, this, "userProperties", (r) => { - const entries = Object.entries(r).map(([k, v]) => [k, new UserPropertyValue(v)]); + copyField(wireFormat, this, "userProperties", (r: unknown) => { + const entries = Object.entries(r as Record) + .filter(([, v]) => { + if (v === null || v === undefined) { + return false; + } + if (typeof v !== "object") { + return false; + } + const vObj = v as Record; + // Filter out empty objects + if (Object.keys(vObj).length === 0) { + return false; + } + // Filter if 'value' property exists but is null/undefined/empty object + if ("value" in vObj) { + const value = vObj.value; + if ( + value === null || + value === undefined || + (typeof value === "object" && value !== null && Object.keys(value).length === 0) + ) { + return false; + } + } + return true; + }) + .map(([k, v]) => [k, new UserPropertyValue(v)]); return Object.fromEntries(entries); }); copyField(wireFormat, this, "bundleInfo", (r) => new ExportBundleInfo(r) as any); @@ -454,9 +480,20 @@ function mapKeys any>( // method always returns a string, similarly to avoid loss of precision, unlike the less-conservative // 'unwrapValue' method just below. /** @hidden */ -function unwrapValueAsString(wrapped: any): string { - const key: string = Object.keys(wrapped)[0]; - return wrapped[key].toString(); +function unwrapValueAsString(wrapped: unknown): string { + if (!wrapped || typeof wrapped !== "object") { + return ""; + } + const keys = Object.keys(wrapped); + if (keys.length === 0) { + return ""; + } + const key: string = keys[0]; + const value = (wrapped as Record)[key]; + if (value === null || value === undefined) { + return ""; + } + return value.toString(); } // Ditto as the method above, but returning the values in the idiomatic JavaScript type (string for strings, From 16b141309c522732888bbacf1e27eaebeb4094f4 Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Mon, 13 Oct 2025 12:58:13 +0100 Subject: [PATCH 2/7] refactor(analytics): clean up filtering code --- src/v1/providers/analytics.ts | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/v1/providers/analytics.ts b/src/v1/providers/analytics.ts index 9136ab293..b60b746d9 100644 --- a/src/v1/providers/analytics.ts +++ b/src/v1/providers/analytics.ts @@ -182,29 +182,19 @@ export class UserDimensions { copyField(wireFormat, this, "userProperties", (r: unknown) => { const entries = Object.entries(r as Record) .filter(([, v]) => { - if (v === null || v === undefined) { + // Property must be a non-empty object. + if (v == null || typeof v !== "object" || Object.keys(v).length === 0) { return false; } - if (typeof v !== "object") { - return false; - } - const vObj = v as Record; - // Filter out empty objects - if (Object.keys(vObj).length === 0) { - return false; + // If 'value' field exists, it must not be null, undefined, or an empty object. + if (!("value" in v)) { + return true; } - // Filter if 'value' property exists but is null/undefined/empty object - if ("value" in vObj) { - const value = vObj.value; - if ( - value === null || - value === undefined || - (typeof value === "object" && value !== null && Object.keys(value).length === 0) - ) { - return false; - } - } - return true; + + const value = (v as { value: unknown }).value; + const isEmptyObject = + typeof value === "object" && value !== null && Object.keys(value).length === 0; + return value != null && !isEmptyObject; }) .map(([k, v]) => [k, new UserPropertyValue(v)]); return Object.fromEntries(entries); From d7068ca1d2cc9a755c0bdc16ed677f6f1d29b1ce Mon Sep 17 00:00:00 2001 From: Jacob Cable Date: Wed, 15 Oct 2025 16:55:09 +0100 Subject: [PATCH 3/7] fix(analytics): check for value field --- spec/v1/providers/analytics.spec.ts | 4 ++++ src/v1/providers/analytics.ts | 9 +++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/spec/v1/providers/analytics.spec.ts b/spec/v1/providers/analytics.spec.ts index 5b5b0c281..2cc1d8ecd 100644 --- a/spec/v1/providers/analytics.spec.ts +++ b/spec/v1/providers/analytics.spec.ts @@ -207,6 +207,9 @@ describe("Analytics Functions", () => { firstOpenTimestampMicros: "577978620000000", userProperties: { foo: { + value: { + stringValue: "bar", + }, setTimestampUsec: "514820220000000", }, }, @@ -236,6 +239,7 @@ describe("Analytics Functions", () => { firstOpenTime: "1988-04-25T13:37:00.000Z", userProperties: { foo: { + value: "bar", setTime: "1986-04-25T13:37:00.000Z", }, }, diff --git a/src/v1/providers/analytics.ts b/src/v1/providers/analytics.ts index b60b746d9..cb418cbc2 100644 --- a/src/v1/providers/analytics.ts +++ b/src/v1/providers/analytics.ts @@ -182,15 +182,12 @@ export class UserDimensions { copyField(wireFormat, this, "userProperties", (r: unknown) => { const entries = Object.entries(r as Record) .filter(([, v]) => { - // Property must be a non-empty object. - if (v == null || typeof v !== "object" || Object.keys(v).length === 0) { + // Property must be an object and have a 'value' field. + if (v == null || typeof v !== "object" || !("value" in v)) { return false; } - // If 'value' field exists, it must not be null, undefined, or an empty object. - if (!("value" in v)) { - return true; - } + // The 'value' field must not be null, undefined, or an empty object. const value = (v as { value: unknown }).value; const isEmptyObject = typeof value === "object" && value !== null && Object.keys(value).length === 0; From beba5f6d765b9db2bb09c7c324fd357519e0e02b Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Mon, 20 Oct 2025 13:44:01 -0700 Subject: [PATCH 4/7] feat: Refactor analytics event handler and update CHANGELOG --- CHANGELOG.md | 7 ++++++- src/v1/providers/analytics.ts | 33 +++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6461e7d1d..54c615f1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1 +1,6 @@ -- Add `defineJsonSecret` API for storing structured JSON objects in Cloud Secret Manager +# Change Log + +## Unreleased + +- Add `defineJsonSecret` API for storing structured JSON objects in Cloud Secret Manager (#1745) +- Refactored analytics event handler to improve readability and maintainability (#1738) \ No newline at end of file diff --git a/src/v1/providers/analytics.ts b/src/v1/providers/analytics.ts index cb418cbc2..d522af00f 100644 --- a/src/v1/providers/analytics.ts +++ b/src/v1/providers/analytics.ts @@ -135,6 +135,26 @@ export class AnalyticsEvent { } } +/** + * @hidden + */ +function isValidUserProperty(property: unknown): property is { value: unknown } { + if (property == null || typeof property !== "object" || !("value" in property)) { + return false; + } + + const { value } = property; + if (value == null) { + return false; + } + + if (typeof value === "object" && Object.keys(value).length === 0) { + return false; + } + + return true; +} + /** * Interface representing the user who triggered the events. */ @@ -181,18 +201,7 @@ export class UserDimensions { this.userProperties = {}; // With no entries in the wire format, present an empty (as opposed to absent) map. copyField(wireFormat, this, "userProperties", (r: unknown) => { const entries = Object.entries(r as Record) - .filter(([, v]) => { - // Property must be an object and have a 'value' field. - if (v == null || typeof v !== "object" || !("value" in v)) { - return false; - } - - // The 'value' field must not be null, undefined, or an empty object. - const value = (v as { value: unknown }).value; - const isEmptyObject = - typeof value === "object" && value !== null && Object.keys(value).length === 0; - return value != null && !isEmptyObject; - }) + .filter(([, v]) => isValidUserProperty(v)) .map(([k, v]) => [k, new UserPropertyValue(v)]); return Object.fromEntries(entries); }); From bbec0c39174d6f74655c37ce8458e825a9241866 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Mon, 20 Oct 2025 13:47:28 -0700 Subject: [PATCH 5/7] reformat changelog. --- CHANGELOG.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54c615f1a..cae941247 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,3 @@ -# Change Log - -## Unreleased - - Add `defineJsonSecret` API for storing structured JSON objects in Cloud Secret Manager (#1745) -- Refactored analytics event handler to improve readability and maintainability (#1738) \ No newline at end of file +- Refactored analytics event handler to improve readability and maintainability (#1738) + From 84eff77e2aa884456d386c2e0945577a01463171 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Mon, 20 Oct 2025 13:48:09 -0700 Subject: [PATCH 6/7] nit: remove unncessary comment. --- src/v1/providers/analytics.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/v1/providers/analytics.ts b/src/v1/providers/analytics.ts index d522af00f..2168df487 100644 --- a/src/v1/providers/analytics.ts +++ b/src/v1/providers/analytics.ts @@ -135,9 +135,6 @@ export class AnalyticsEvent { } } -/** - * @hidden - */ function isValidUserProperty(property: unknown): property is { value: unknown } { if (property == null || typeof property !== "object" || !("value" in property)) { return false; From 32207e45b937d7a2b1f55084b58dee8a3f7a9552 Mon Sep 17 00:00:00 2001 From: Daniel Lee Date: Mon, 20 Oct 2025 13:49:00 -0700 Subject: [PATCH 7/7] reword changelog. --- CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cae941247..c0a8ece25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,2 @@ -- Add `defineJsonSecret` API for storing structured JSON objects in Cloud Secret Manager (#1745) -- Refactored analytics event handler to improve readability and maintainability (#1738) - +- Add `defineJsonSecret` API for storing structured JSON objects in Cloud Secret Manager. (#1745) +- Enhance validation against incomplete/invalid app_remove events to avoid runtime crashes. (#1738)