Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
90 changes: 90 additions & 0 deletions spec/v1/providers/analytics.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ describe("Analytics Functions", () => {
firstOpenTimestampMicros: "577978620000000",
userProperties: {
foo: {
value: {
stringValue: "bar",
},
setTimestampUsec: "514820220000000",
},
},
Expand Down Expand Up @@ -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",
},
},
Expand Down Expand Up @@ -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",
},
},
},
});
});
});
});

Expand Down
40 changes: 35 additions & 5 deletions src/v1/providers/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
Loading