Skip to content

Parse Server Crash on invalid/expired APNS cert #3575

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

Open
gateway opened this issue Feb 27, 2017 · 15 comments
Open

Parse Server Crash on invalid/expired APNS cert #3575

gateway opened this issue Feb 27, 2017 · 15 comments
Labels
type:feature New feature or improvement of existing feature

Comments

@gateway
Copy link

gateway commented Feb 27, 2017

We use GitHub Issues for bugs.

If you have a non-bug question, ask on Stack Overflow or Server Fault:

You may also search through existing issues before opening a new one: https://github.com/ParsePlatform/Parse-Server/issues?utf8=%E2%9C%93&q=is%3Aissue

--- Please use this template. If you don't use this template, your issue may be closed without comment. ---

Issue Description

Parse server 2.3.6 crashes when the push cert for apns has expired.

Describe your issue in as much detail as possible.

Steps to reproduce

Get put a old apns cert in and log in as a new user.

Actual Outcome

What is happening instead.

Environment Setup

  • Server

    • parse-server version (Be specific! Don't say 'latest'.) : 2.3.6
    • Operating System: Heroku
    • Hardware: Standard 2x
    • Localhost or remote server? Heroku
  • Database

    • MongoDB version: 3.2
    • Storage engine: tiger
    • Hardware: dedicated
    • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): mlab.

Logs/Trace

Feb 27 11:31:00 stage app/web.1: ERR! parse-server-push-adapter APNS cannot find vaild connection for c867c8504a4657fe77070971d11644ab84f95105524a632c6b5b2738957f7e6d
Feb 27 11:31:00 stage app/web.1: /app/node_modules/parse-server/lib/ParseServer.js:460
Feb 27 11:31:00 stage app/web.1: throw err;
Feb 27 11:31:00 stage app/web.1: ^
Feb 27 11:31:00 stage app/web.1: Error: certificate has expired: 2017-02-10T00:33:23.000Z
Feb 27 11:31:00 stage app/web.1: at validateCredentials (/app/node_modules/apn/lib/credentials/validate.js:12:9)
Feb 27 11:31:00 stage app/web.1: at /app/node_modules/apn/lib/connection.js:163:5
Feb 27 11:31:00 stage app/web.1: at _fulfilled (/app/node_modules/q/q.js:834:54)
Feb 27 11:31:00 stage app/web.1: at self.promiseDispatch.done (/app/node_modules/q/q.js:863:30)
Feb 27 11:31:00 stage app/web.1: at Promise.promise.promiseDispatch (/app/node_modules/q/q.js:796:13)
Feb 27 11:31:00 stage app/web.1: at /app/node_modules/q/q.js:604:44
Feb 27 11:31:00 stage app/web.1: at runSingle (/app/node_modules/q/q.js:137:13)
Feb 27 11:31:00 stage app/web.1: at flush (/app/node_modules/q/q.js:125:13)
Feb 27 11:31:00 stage app/web.1: at _combinedTickCallback (internal/process/next_tick.js:67:7)
Feb 27 11:31:00 stage app/web.1: at process._tickDomainCallback (internal/process/next_tick.js:122:9)
Feb 27 11:31:00 stage app/web.1: npm ERR! Linux 3.13.0-105-generic
Feb 27 11:31:00 stage app/web.1: npm ERR! argv "/app/.heroku/node/bin/node" "/app/.heroku/node/bin/npm" "start"
Feb 27 11:31:00 stage app/web.1: npm ERR! node v7.6.0
Feb 27 11:31:00 stage app/web.1: npm ERR! npm v4.1.2
Feb 27 11:31:00 stage app/web.1: npm ERR! code ELIFECYCLE
Feb 27 11:31:00 stage app/web.1: npm ERR! [email protected] start: node index.js
Feb 27 11:31:00 stage app/web.1: npm ERR! Exit status 7
Feb 27 11:31:00 stage app/web.1: npm ERR!
Feb 27 11:31:00 stage app/web.1: npm ERR! Failed at the [email protected] start script 'node index.js'.
Feb 27 11:31:00 stage app/web.1: npm ERR! Make sure you have the latest version of node.js and npm installed.
Feb 27 11:31:00 stage app/web.1: npm ERR! If you do, this is most likely a problem with the parse-server-example package,
Feb 27 11:31:00 stage app/web.1: npm ERR! not with npm itself.
Feb 27 11:31:00 stage app/web.1: npm ERR! Tell the author that this fails on your system:
Feb 27 11:31:00 stage app/web.1: npm ERR! node index.js
Feb 27 11:31:00 stage app/web.1: npm ERR! You can get information on how to open an issue for this project with:
Feb 27 11:31:00 stage app/web.1: npm ERR! npm bugs parse-server-example
Feb 27 11:31:00 stage app/web.1: npm ERR! Or if that isn't available, you can get their info via:
Feb 27 11:31:00 stage app/web.1: npm ERR! npm owner ls parse-server-example
Feb 27 11:31:00 stage app/web.1: npm ERR! There is likely additional logging output above.
Feb 27 11:31:00 stage app/web.1: npm ERR! Please include the following file with any support request:
Feb 27 11:31:00 stage app/web.1: npm ERR! /app/npm-debug.log

@acinader
Copy link
Contributor

My $.02 is that this is pretty good behavior. The error message is very clear.

An improvement would be to fail fast. I.e. we could validate the apns cert at startup if configured and fail with the above error right away rather than waiting for a push?

What did you have in mind? I don't really like the idea of logging a warning and carrying on. Again, my $.02 and I'm interested in what others think....

@gateway
Copy link
Author

gateway commented Feb 27, 2017

Well it crashes the server for me which I wasn't expecting. Also if your haven't restarted the server in a while you will never potentially see its expired , or will it spit something out when a push tries to go out?

Its the first time I saw this and its not a huge deal since its our staging server but thought I would bring it up for a conversation.

@funkenstrahlen
Copy link
Contributor

Maybe this should better be discussed in the parse-server-push-adapter repository?

@bnol
Copy link

bnol commented Mar 15, 2017

+1

@funkenstrahlen
Copy link
Contributor

I also think this should just throw an error but not crash the server. It is a problem if push notifications do not get send out but it should not crash all other server functionality.

@bnol
Copy link

bnol commented Mar 15, 2017

Any plan working on this issue yet? =/

@bnol
Copy link

bnol commented Mar 15, 2017

@funkenstrahlen you're a contributor of parse-server-push-adapter, aren't you? This error happens in parse-server-push-adapter which is using a very old release of node-apn (1.2.0, 2.1.3 is the current release). Any plan for updating it? :)

UPDATE: I also opened an issue on parse-server-push-adapter project :)

@funkenstrahlen
Copy link
Contributor

@bnol There is already a PR for updating the node-apn: parse-community/parse-server-push-adapter#52

@flovilmart
Copy link
Contributor

@bnol, apn 1.2 is not old and is correctly working. I believe the package throws and for a good reason. You should probably open an issue on the apn package repository if you believe this is an issue.

@bnol
Copy link

bnol commented Mar 15, 2017

@flovilmart the problem is we have no better way to handle that error for now (to avoid the server to be crashed). Or is there any way to work around with it?

@flovilmart
Copy link
Contributor

Have a valid certificate?

@bnol
Copy link

bnol commented Mar 15, 2017

@flovilmart we sure will update the cert. But in this case, crashing the whole process should never be an expected behaviour. I think we could make it better. :)

@flovilmart
Copy link
Contributor

Not sure that's our responsibility in this module, probably in APNs node-apn/node-apn#541

Follow the discussion here and open a PR if you need too.

@flovilmart
Copy link
Contributor

Closing as the crash occurs in the node apn library

@mtrezza
Copy link
Member

mtrezza commented Nov 7, 2020

Reopening this issue to look into whether

  • the newest APN lib still crashes on expired cert
  • there is a way to handle the crashing APN lib gracefully within Parse Server
  • there is another way to notify a user of an expired certificate other than crashing the server (e.g. a new Parse Dashboard Server Status Page)

Generally I agree that an expired cert should not crash the server, if there is another, at least equally effective way to notify the developer. The business impact of a crashed server may be higher than the impact of not sending push notifications. In other use cases, where sending push notifications is an essential part of the app, it may be equally impactful like a crashed server. So we'd have to make sure there is an effective way of notifying the developer.

@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

6 participants