diff --git a/CHANGELOG.md b/CHANGELOG.md index 6461e7d1d..c0a8ece25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1 +1,2 @@ -- Add `defineJsonSecret` API for storing structured JSON objects in Cloud Secret Manager +- 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) diff --git a/spec/v1/providers/analytics.spec.ts b/spec/v1/providers/analytics.spec.ts index 98db1702f..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", }, }, @@ -301,6 +305,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..2168df487 100644 --- a/src/v1/providers/analytics.ts +++ b/src/v1/providers/analytics.ts @@ -135,6 +135,23 @@ export class AnalyticsEvent { } } +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. */ @@ -179,8 +196,10 @@ 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]) => isValidUserProperty(v)) + .map(([k, v]) => [k, new UserPropertyValue(v)]); return Object.fromEntries(entries); }); copyField(wireFormat, this, "bundleInfo", (r) => new ExportBundleInfo(r) as any); @@ -454,9 +473,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,