Investigate Sentry error `Cannot use a string pattern on a bytes-like object`

[DMRobertson]

First seen a month ago --- did we do some kind of release then?

Internal users: https://sentry.matrix.org/sentry/sygnal-matrixorg/issues/228100/?query=is%3Aunresolved

TypeError: cannot use a string pattern on a bytes-like object
  File "sygnal/http.py", line 265, in _handle_dispatch
    result = await pushkin.dispatch_notification(notif, d, context)
  File "sygnal/notifications.py", line 180, in dispatch_notification
    return await self._dispatch_notification_unlimited(n, device, context)
  File "sygnal/webpushpushkin.py", line 151, in _dispatch_notification_unlimited
    regex.fullmatch(endpoint_domain) for regex in self.allowed_endpoints
  File "sygnal/webpushpushkin.py", line 151, in <genexpr>
    regex.fullmatch(endpoint_domain) for regex in self.allowed_endpoints

Need to understand:

• which regex are we using and where did it come from
• ditto for endpoint_domain

[DMRobertson]

• endpoint_domain is urlparse(endpoint).netloc. It's a str or bytes depending on the type of endpoint.
• endpoint is device.data.get("endpoint").
• device is a Device. device.data is either None or raw["data"]
• I'm assuming that raw is a dict coming from some json and so should contain strings rather than bytes.
• So perhaps the program constructs a raw by itself somewhere?

[DMRobertson]

Oh, I'm an idiot. Some of the earlier sentry events have the locals on them (most recent one didn't seem to parse somehow?)

endpoint_domain is b''.

[DMRobertson]

appId often seems to be hydrogen: io.element.hydrogen.web

[clokep]

• I'm assuming that raw is a dict coming from some json and so should contain strings rather than bytes.

• Devices get built from notif["devices"] in Notification
• Notifications get built via body = json_decoder.decode(request.content.read().decode("utf-8")) -> Notification(body["notification"])

So I think I agree with you that this should be a string, not bytes.

Note that it seems that urlparse has some magic in it though:

>>> from urllib.parse import urlparse
>>> urlparse(None)
ParseResultBytes(scheme=b'', netloc=b'', path=b'', params=b'', query=b'', fragment=b'')

This seems to match the data we're getting (endpoint == None).

[clokep]

(I think in this case allowed can be set to false directly?) @bwindels might have some other thoughts though.

[DMRobertson]

Note that it seems that urlparse has some magic in it though:

Bingo.

That does match the type stub. So I guess for us to have detected this we'd need device.data to be more strongly typed.

The spec says that data is optional, but there's a note about http pushers which suggests that data might be required for an http pusher? Can investigate more tomorrow.

[clokep]

The spec says that data is optional, but there's a note about http pushers which suggests that data might be required for an http pusher? Can investigate more tomorrow.

data seems to exist OK though, I think the issue is the endpoint key is missing in it.

[squahtx]

data seems to exist OK though, I think the issue is the endpoint key is missing in it.

That seems like what's happening.

We have nowhere to deliver the notification if the pushkey is missing an endpoint, so I'm going to make sygnal reject the pushkey.