Page MenuHomePhabricator

Fix payloadSizeExceeded calculated incorrectly
ClosedPublic

Authored by angelika on Oct 15 2024, 5:05 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 18, 8:38 AM
Unknown Object (File)
Wed, Dec 18, 8:38 AM
Unknown Object (File)
Wed, Dec 18, 8:38 AM
Unknown Object (File)
Wed, Dec 18, 8:38 AM
Unknown Object (File)
Wed, Dec 18, 8:37 AM
Unknown Object (File)
Thu, Dec 5, 5:45 AM
Unknown Object (File)
Sun, Dec 1, 2:44 AM
Unknown Object (File)
Tue, Nov 26, 2:27 AM
Subscribers

Details

Summary

notificationSizeValidator returns true if notification size is less than maximum notification size (that is, the size is correct). However we set payloadSizeExceeded
to true if notificationSizeValidator returns true. That's obviously wrong and we need to negate it.
I think the bug was introduced in the first place by notificationSizeValidator being not obvious name. I renamed it in another diff (D13713).

Test Plan
  1. Create a new chat
  2. Verify that notification is verified correctly, that is payloadSizeExceeded is false.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat requested changes to this revision.Oct 15 2024, 9:30 AM

Nice work! It looks like Android has the same issue, but the variable is named slightly differently. If you agree with my assessment, can you fix it for Android too?

lib/push/crypto.js
55 ↗(On Diff #45194)

How about here?

98 ↗(On Diff #45194)

How about here?

This revision now requires changes to proceed.Oct 15 2024, 9:30 AM
This revision is now accepted and ready to land.Oct 15 2024, 2:53 PM