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

FCM/ADM router notification size limit check is inaccurate #203

Closed
AzureMarker opened this issue Jul 27, 2020 · 1 comment · Fixed by #212 or #231
Closed

FCM/ADM router notification size limit check is inaccurate #203

AzureMarker opened this issue Jul 27, 2020 · 1 comment · Fixed by #212 or #231
Labels
2 Estimate - Small

Comments

@AzureMarker
Copy link
Contributor

AzureMarker commented Jul 27, 2020

The FCM router (and any router which uses the common build_message code) only checks the notification size limit against the data that is being passed through, not the final payload:

if data.len() > max_data {

This check could allow payloads which have a small enough data size but too much overall data in the final payload. The check should be moved so it is done on the final payload.

@AzureMarker AzureMarker added the 2 Estimate - Small label Jul 27, 2020
@AzureMarker AzureMarker added this to the Autoendpoint Rust Server milestone Jul 27, 2020
@AzureMarker AzureMarker added this to Backlog: Misc in Services Engineering via automation Jul 27, 2020
@AzureMarker AzureMarker moved this from Backlog: Misc to Backlog: Push in Services Engineering Jul 27, 2020
@AzureMarker AzureMarker changed the title FCM router notification size limit check is too conservative FCM/ADM router notification size limit check is inaccurate Aug 4, 2020
@AzureMarker AzureMarker moved this from Backlog: Push to In Progress in Services Engineering Aug 7, 2020
@AzureMarker AzureMarker moved this from In Progress to In Review in Services Engineering Aug 7, 2020
@AzureMarker
Copy link
Contributor Author

From testing, FCM only considers the data field when calculating the payload size.

Services Engineering automation moved this from In Review to Done Aug 10, 2020
AzureMarker added a commit that referenced this issue Aug 10, 2020
* Check the max data size against the final message payload

* Use a more accurate size check for FCM messages

Fixes #203
@tublitzed tublitzed moved this from Done to Archived in Services Engineering Aug 17, 2020
This was referenced Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment