Skip to content
Draft
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
12 changes: 9 additions & 3 deletions src/firebase_functions/pubsub_fn.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ class MessagePublishedData(_typing.Generic[T]):
Google Cloud Pub/Sub message.
"""

subscription: str
subscription: str | None
"""
A subscription resource.
A subscription resource. ``None`` when the request had no CloudEvent headers.
"""


Expand All @@ -107,6 +107,12 @@ def _message_handler(
data = event_dict["data"]
message_dict = data["message"]

# When a request has no CloudEvent headers, functions-framework builds the
# message itself with camelCase keys only, an "@type" key and no subscription.
message_dict.setdefault("message_id", message_dict.get("messageId"))
message_dict.setdefault("publish_time", message_dict.get("publishTime"))
Comment on lines +112 to +113

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using message_dict.get() here will return None if the camelCase keys are missing. If both the snake_case and camelCase keys are absent (e.g., in a malformed request), setdefault will set the keys to None. This can lead to confusing downstream errors, such as a TypeError: argument of type 'NoneType' is not iterable when checking for . in publish_time on line 119, instead of a standard KeyError. Checking for the existence of the camelCase keys before calling setdefault avoids injecting None values.

Suggested change
message_dict.setdefault("message_id", message_dict.get("messageId"))
message_dict.setdefault("publish_time", message_dict.get("publishTime"))
if "messageId" in message_dict:
message_dict.setdefault("message_id", message_dict["messageId"])
if "publishTime" in message_dict:
message_dict.setdefault("publish_time", message_dict["publishTime"])

message_dict.pop("@type", None)

# if no microseconds are present, we should set them to 0 to prevent parsing from failing
if "." not in event_dict["time"]:
event_dict["time"] = event_dict["time"].replace("Z", ".000000Z")
Expand Down Expand Up @@ -147,7 +153,7 @@ def _message_handler(
**message_dict,
ordering_key=ordering_key,
),
subscription=data["subscription"],
subscription=data.get("subscription"),
)

event_dict["data"] = message
Expand Down
28 changes: 28 additions & 0 deletions tests/test_pubsub_fn.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from unittest.mock import MagicMock

from cloudevents.http import CloudEvent as _CloudEvent
from flask import Flask, request
from functions_framework import event_conversion

from firebase_functions import core
from firebase_functions.pubsub_fn import (
Expand Down Expand Up @@ -182,3 +184,29 @@ def test_publish_time_is_timezone_aware_datetime(self):
_dt.datetime(2023, 3, 11, 13, 25, 37, 403000, tzinfo=_dt.timezone.utc),
)
self.assertEqual(message.publish_time.utcoffset(), _dt.timedelta(0))

def test_push_body_without_cloudevent_headers_reaches_handler(self):
push_body = {
"message": {
"attributes": {"key": "value"},
"data": "eyJ0ZXN0IjogInZhbHVlIn0=",
"messageId": "message-id-123",
"publishTime": "2023-03-11T13:25:37.403Z",
},
"subscription": "projects/p/subscriptions/s",
}
with Flask(__name__).test_request_context("/", method="POST", json=push_body):
raw = event_conversion.background_event_to_cloud_event(request)

func = MagicMock()
_message_handler(func, raw)

data = func.call_args.args[0].data
self.assertEqual(data.message.message_id, "message-id-123")
self.assertEqual(
data.message.publish_time,
_dt.datetime(2023, 3, 11, 13, 25, 37, 403000, tzinfo=_dt.timezone.utc),
)
self.assertEqual(data.message.attributes, {"key": "value"})
self.assertEqual(data.message.json, {"test": "value"})
self.assertIsNone(data.subscription)
Loading