fix(analytics): validate against incomplete/invalid app_remove events (#1738)

* fix(analytics): validate against incomplete/invalid app_remove events

* refactor(analytics): clean up filtering code

* fix(analytics): check for value field

* feat: Refactor analytics event handler and update CHANGELOG

* reformat changelog.

* nit: remove unncessary comment.

* reword changelog.

---------

Co-authored-by: Daniel Lee <[email protected]>

Author: cabljac

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)

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",
+              },
+            },
+          },
+        });
+      });
     });
   });
 

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<string, unknown>)
+        .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<Obj, Transform extends (key: keyof Obj) => 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<string, unknown>)[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,