pushy icon indicating copy to clipboard operation
pushy copied to clipboard

ValidatingPushNotificationHandler not verifying expirationTimestamp

Open brandonm opened this issue 4 years ago • 2 comments

https://github.com/jchambers/pushy/blob/e67df92b35998767a3880f438158537c997cf8dc/pushy/src/main/java/com/eatthepath/pushy/apns/server/ValidatingPushNotificationHandler.java#L139-L143

Is there a reason why the code is not actually verifying if it's expired but assuming if it exists then its expired?

Comment in ValidatingPushNotificationHandlerFactory.class makes it sound like it will be checked.

 * @param expirationTimestampsByDeviceToken a map of device tokens to the time at which they expire; tokens not in
 * the map (or all tokens if the map is {@code null} or empty) will never be considered "expired"

brandonm avatar Jul 28 '21 18:07 brandonm

At a glance, that looks like it's just a mistake. I'll take a closer look shortly.

jchambers avatar Jul 29 '21 03:07 jchambers

I've looked more closely at this, and this is behaving as expected, though I recognize that the documentation could be clearer. The design intent is that callers can ask the mock server to report that a device token expired at a certain timestamp. The idea is that the timestamp should be in the past, but it's not technically required.

The check you referenced is essentially asking "has somebody specified that this device is expired?" and not "should I perform some time check to see if it is expired?"

I'll think about some ways to rephrase the docs to be clearer, but I do not believe there's any functional problem here.

jchambers avatar Jan 15 '24 22:01 jchambers