Skip to content

Conversation

@p1gp1g
Copy link
Collaborator

@p1gp1g p1gp1g commented Oct 30, 2025

This PR fixes the parsing of web push device tokens: the keys and endpoint are parsed during registration, which avoid late failure.

It also adds tests for the decoding, and the encryption, using the RFC8291 example.

There are still some todo in the comments, but I prefer to see the different points before removing them:

I haven't changed how the push notification is encoded for webpush (the todo suggests to move to ToJSON). I had the implementation, but I preferred to discuss it first. The reasons are:

  • APNS do not use ToJSON for push notifications too, so we're following APNS on this
  • We could move APNS to use ToJSON too. But this would lead to breaking changes
  • APNS needs to add some other field to the JSON (like the nonce)
  • We don't need to actually use ToJSON because we don't restore the full notification. We either use the verification code, or just synchronize on push (and ignore the content) (I think this is the most important point)
  • We can have a ToJSON that actually does what encodePN does - but then if at some point we need to decode them, it wouldn't be possible anymore. And we can derive with defaultJSON, but the default format uses a lot of unnecessary data, and what's the best way to encode the (and what the best the SMPQueueNtf ? strEncoding it, or implementing ToJSON for it too ?)

apnsPushProviderClient and wpPushProviderClient do not restrict during compile time to a defined PushProvider type and fails with PPInvalidPusher if it is used with the wrong provider. It's because these functions are used in a single place which is easily controlled that we don't mix the different pushers. Doing the restriction during compilation would require introducing some GADTS which would add a lot of complexity to the code for an arguable benefit

@p1gp1g p1gp1g changed the title Wp parsing WebPush parsing Oct 30, 2025
@p1gp1g p1gp1g changed the title WebPush parsing [webpush] Parsing during registration Oct 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant