Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APNS HTTP API with same cert/key as the binary API. Based on #149 #173

Merged
merged 29 commits into from
Jul 8, 2017

Conversation

TysonAndre
Copy link
Contributor

This requires calling /addpsp again, and providing bundleid in addition
to pre-existing parameters.

  • This will be the same PSP, and subscriptions will be unaffected
  • Aside: If calling /addpsp, all fields must be provided again.
    E.g. for staging/debug(sandbox) applications, sandbox=true must be provided yet again.

It would currently be difficult (inconvenient, and possibly backwards incompatible, but not impossible)
to migrate old subscribers from cert/key to p8

  • without further changes to the way FixedData is hashed,
    p8 would be currently be treated as a completely different PSP with a different subscriber set

Store the bundleid in VolatileData, so that the computed hash of the PSP
remains the same, and existing subscriptions will work.

Fix test failures, still need to allow requests to specify http2

allow specifying uniqush.http2=1 in query params for per-request APNS override
(Later, when this is well tested and performance change is known,
this may be in a config or always done when bundleid is known)

allow multiple APNS HTTP2 PSPs with different certificates
(With the p8 approach, that wouldn't be a problem, but with client
certificates, it is)

Finish getting the HTTP2 protocol to work

Disable IdleTimeout (the same as GCM). Timing out after 90 seconds would make latency unpredictable, and may cause errors when pushes are infrequent or sporadic.


Bug fixes:

  • Fix unsubscribing with the http2 provider, make it send the same Status (STATUS8_UNSUBSCRIBE) as the binary provider would
  • Change timestamp to be an int64 - Some responses failed to unserialize because the timestamp couldn't be converted from a number to a string in the struct.

dhanui and others added 29 commits June 13, 2017 15:47
This requires calling /addpsp again, and providing bundleid in addition
to pre-existing parameters.

- It would currently be impossible to migrate old subscribers from
  cert/key to p8
  (without further changes to the way FixedData is hashed)

  Store the bundleid in VolatileData, so that the computed hash of the PSP
  remains the same, and existing subscriptions will work.

  Note: I think spps might change the name of their bundle id (but keep
  subscribers) if they change ownership, which is why this is considered VolatileData.
  Not 100% sure of that. Anyway, if multiple apps ever have the same certificate
  data, then give them different service names or different paths
  (to files with identical certificate contents)
- Fix test failures, still need to allow requests to specify http2
- allow specifying `uniqush.http2=1` in query params for per-request APNS override
  (Later, when this is well tested and performance change is known,
  this may be in a config or always done when bundleid is known)
- allow multiple APNS HTTP2 PSPs with different certificates
  (With the p8 approach, that wouldn't be a problem, but with client
  certificates, it is)

  This may be refactored later to be more like the binary protocol data.
- Finish getting the HTTP2 protocol to work

TODO: Test this with `/shutdown`.
TODO: Compare performance and network usage of http2 and binary protocol.
Validate header values are the expected values.
This isn't comprehensive.
I suspect that the idle timeout may be related to these errors,
since pushes are infrequent on stage.

HTTP2 may allow 100s of "requests" simultaneously on the same
HTTP2 connection, but that depends on apple's configuration.
The initial version failed to send unsubscribe updates on resChan,
which is something the binary API would do.
The binary provider API sends status codes to push_service.go
(From a byte of the TCP response with the associated message id)

In order for uniqush to properly unsubscribe, it must send the same
information over resChan, so that push_service.go can unsubscribe.

Long term, the abstraction would probably be flipped
(Make an HTTPClient interface out of the binary provider API)

Add constant definitions for the binary API's bytes.
@TysonAndre TysonAndre requested a review from mishan July 7, 2017 18:49
@TysonAndre TysonAndre merged commit d8c99b4 into master Jul 8, 2017
TysonAndre added a commit that referenced this pull request Jul 14, 2017
+ New feature: Add /previewpush endpoint to preview the payload that would be
  generated and sent to push services. (Issue #140)
  This helps with debugging.
+ Maintenance: Update APNS binary provider API(default) from version 1 to version 2.
+ Maintenance: Upgrade to redis.v5 (Issue #143)
+ New provider: Add FCM support. (Issue #148)
  The parameters that would be provided to /addpsp, /subscribe, and /push are
  the same as they would be for GCM. (Replace "fcm" with "gcm")
+ New feature: Add support APNS HTTP2 API (Issue #157, PR #173)
  This gives more accurate results on whether a push succeeded,
  and should not impact Uniqush's performance.
  To set this up, call /addpsp (to create a new provider or modify an
  existing provider) with the same params you would use to create a new
  APNS endpoint for binary providers (including cert and key),
  in addition to providing `bundleid`.
  Currently, to make testing easy, each call to `/push` must be provided with
  the query param value `uniqush.http=1`.
  Otherwise, uniqush continues to use the APNS binary provider API.
+ Maintenance: Use unescaped payloads for GCM and FCM.
  This allows larger payloads, avoiding escaping characters such as `<` and `>`

Fixes #134

go 1.8.3+ and an up to date version of golang.org/x/net/http2
are suggested (For the APNS HTTP2 API).
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.

None yet

3 participants