Hotfix: Ignore invalid expire timer sync resets #2086
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
iOS inadvertently sends `null` `expireTimer` for groups during contact sync. Ignore these resets until we have fixed the issue. See: #2079
michaelkirk
reviewed
Feb 28, 2018
js/background.js
Outdated
| // TODO: Remove once | ||
| // https://github.com/signalapp/Signal-Desktop/issues/2079 | ||
| // is resolved: | ||
| const isInvalidIOSExpireTimerReset = expireTimer === null; |
There was a problem hiding this comment.
Does this work?
My understanding of the iOS bug was not that we set the field with a weird value, but that we just never set it.
There was a problem hiding this comment.
Here's a link to the diff:
In the future, we will use zero (`0`) to denote disabling of expire timer.
gasi
commented
Mar 1, 2018
| source, | ||
| receivedAt, | ||
| {fromSync: true} | ||
| ); |
There was a problem hiding this comment.
We should dedupe the two identical code snippets above but I didn’t to reduce risk for the hotfix.
|
Tested this with Signal iOS 2.19.6 and Signal Android 4.15.5 and it ignores (and logs) all non-numeric |
gasi
changed the title
scottnonnenberg
approved these changes
Mar 1, 2018
scottnonnenberg
added a commit
that referenced
this pull request
Mar 1, 2018
scottnonnenberg
added a commit
that referenced
this pull request
Mar 1, 2018
Note: This release is equivalent to v1.5.2 Fixed: If interrupted in the middle of an import, next registration would leave that imported data in the database (#2072) Fixed: In some cases on OSX, View menu would not have Debug Log option (#2089) Fixed: In import/registration flow, choosing View -> Debug Log would do nothing (#2089) Fixed: In some cases contact/group syncs would turn off disappearing messages in all conversations (#2086) Fixed: On initial setup, conversations with disappearing messages enabled would be at the top of the conversation list (#2084)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
iOS omits
expireTimerprotobuf property to denote disappearing messages have been turned off. However, that doesn’t allow us to distinguish it from old clients that are not aware of this property. This change ignores these invalid values until we consistently use0to denote disabled disappearing messages.expireTimervalues during contact sync. Long-term, we’ll use0to denote turning off expire timers.expireTimeris set to.expireTimerinhandleDataMessageuntil it usesConversationController::updateExpirationTimer.