From d24c184f5a41df6058a5fa78fe331297d5a4f83e Mon Sep 17 00:00:00 2001
From: Sean Quah <seanq@element.io>
Date: Wed, 8 Dec 2021 18:04:53 +0000
Subject: [PATCH 1/4] Reject web pushkeys with missing endpoints instead of
 raising an error

---
 changelog.d/288.bugfix   |  1 +
 sygnal/webpushpushkin.py | 14 ++++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/288.bugfix

diff --git a/changelog.d/288.bugfix b/changelog.d/288.bugfix
new file mode 100644
index 00000000..b633f9b0
--- /dev/null
+++ b/changelog.d/288.bugfix
@@ -0,0 +1 @@
+Reject web pushkeys with missing endpoints instead of raising an error.
diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py
index fea3c494..3a179037 100644
--- a/sygnal/webpushpushkin.py
+++ b/sygnal/webpushpushkin.py
@@ -114,7 +114,7 @@ def __init__(self, name: str, sygnal: "Sygnal", config: Dict[str, Any]):
         )
         self.http_request_factory = HttpRequestFactory()
 
-        self.allowed_endpoints = None  # type: Optional[List[Pattern]]
+        self.allowed_endpoints: Optional[List[Pattern[str]]] = None
         allowed_endpoints = self.get_config("allowed_endpoints", list)
         if allowed_endpoints:
             self.allowed_endpoints = list(map(glob_to_regex, allowed_endpoints))
@@ -149,6 +149,16 @@ async def _dispatch_notification_unlimited(
             return []
 
         endpoint = device.data.get("endpoint")
+        if not isinstance(endpoint, str):
+            # The pushkey is missing the endpoint for delivery.
+            logger.warn(
+                "Rejecting pushkey %s; "
+                "device.data.endpoint is missing or not a string: %r",
+                device.pushkey,
+                endpoint,
+            )
+            return [device.pushkey]
+
         auth = device.data.get("auth")
         endpoint_domain = urlparse(endpoint).netloc
         if self.allowed_endpoints:
@@ -289,7 +299,7 @@ def _handle_response(
         response: IResponse,
         response_text: str,
         pushkey: str,
-        endpoint_domain: bytes,
+        endpoint_domain: str,
     ) -> bool:
         """
         Logs and determines the outcome of the response

From 171e15beb20157f20986fb3d05f8b69d49db5a90 Mon Sep 17 00:00:00 2001
From: Sean Quah <seanq@element.io>
Date: Wed, 8 Dec 2021 20:03:59 +0000
Subject: [PATCH 2/4] Fix typo in docs

---
 docs/applications.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/applications.md b/docs/applications.md
index f04628a5..b102d840 100644
--- a/docs/applications.md
+++ b/docs/applications.md
@@ -247,7 +247,7 @@ In your web application, [the push manager subscribe method](https://developer.m
 will return
 [a subscription](https://developer.mozilla.org/en-US/docs/Web/API/PushSubscription) 
 with an `endpoint` and `keys` property, the latter containing a `p256dh` and `auth` 
-property. The `p256dh` key is used as the push key, and the push data is expected 
+property. The `p256dh` key is used as the push key, and the push data must contain
 `endpoint` and `auth`. You can also set `default_payload` in the push data;
 any properties set in it will be present in the push messages you receive, 
 so it can be used to pass identifiers specific to your client

From 6b98fd2a4cb8da537b2318f887e4c58cfc8c11f4 Mon Sep 17 00:00:00 2001
From: Sean Quah <seanq@element.io>
Date: Wed, 8 Dec 2021 20:06:48 +0000
Subject: [PATCH 3/4] Better fix

---
 sygnal/webpushpushkin.py | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/sygnal/webpushpushkin.py b/sygnal/webpushpushkin.py
index 3a179037..da42629d 100644
--- a/sygnal/webpushpushkin.py
+++ b/sygnal/webpushpushkin.py
@@ -149,17 +149,18 @@ async def _dispatch_notification_unlimited(
             return []
 
         endpoint = device.data.get("endpoint")
-        if not isinstance(endpoint, str):
-            # The pushkey is missing the endpoint for delivery.
+        auth = device.data.get("auth")
+
+        if not p256dh or not isinstance(endpoint, str) or not isinstance(auth, str):
             logger.warn(
-                "Rejecting pushkey %s; "
-                "device.data.endpoint is missing or not a string: %r",
-                device.pushkey,
+                "Rejecting pushkey; subscription info incomplete or invalid "
+                + "(p256dh: %s, endpoint: %r, auth: %r)",
+                p256dh,
                 endpoint,
+                auth,
             )
             return [device.pushkey]
 
-        auth = device.data.get("auth")
         endpoint_domain = urlparse(endpoint).netloc
         if self.allowed_endpoints:
             allowed = any(
@@ -173,16 +174,6 @@ async def _dispatch_notification_unlimited(
                 # abort, but don't reject push key
                 return []
 
-        if not p256dh or not endpoint or not auth:
-            logger.warn(
-                "Rejecting pushkey; subscription info incomplete "
-                + "(p256dh: %s, endpoint: %s, auth: %s)",
-                p256dh,
-                endpoint,
-                auth,
-            )
-            return [device.pushkey]
-
         subscription_info = {
             "endpoint": endpoint,
             "keys": {"p256dh": p256dh, "auth": auth},

From 88f08896c50f487e9ab627731c0f9fbbd56d0aef Mon Sep 17 00:00:00 2001
From: Sean Quah <seanq@element.io>
Date: Wed, 8 Dec 2021 20:10:13 +0000
Subject: [PATCH 4/4] Update newsfile

---
 changelog.d/288.bugfix | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/changelog.d/288.bugfix b/changelog.d/288.bugfix
index b633f9b0..80470ece 100644
--- a/changelog.d/288.bugfix
+++ b/changelog.d/288.bugfix
@@ -1 +1 @@
-Reject web pushkeys with missing endpoints instead of raising an error.
+Fix a bug introduced in Sygnal 0.9.1 where web pushkeys with missing endpoints would cause an error.