-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Notification.Tag probably shouldn't be serialized. #678
Comments
This is actually a VERY good spot. I actually am having an issue right now where if I provide a Tag, the GCMServiceBroker breaks and will not send messages or stop. |
This is a great catch. There is no need to serialize this property that I can think of. If it's something you absolutely want to do, you can encapsulate the notification class inside of your own object. I will be adding |
Awesome @Redth. Was just about to work on a pull request for it. If possible can this please be released as 4.0.5? |
it will be in the very next release shortly. And to clarify, really only GcmNotification needs this treatment, as I don't use the json serializer for the other platforms. Eventually Json.Net may be removed as a dependency, but for now this is a good change. |
Thanks! That was quick. :) |
Currently Notification.Tag on all the different platforms has no [JsonIgnore] attribute and it actually gets serialized and sent to the Push Notification Service. This leads to subtle bugs, such as when you have some non-serializable attributes which just make Json.Net crash. It can also leak out unintended data, since users don't expect it to be sent to APNS/GCM/etc.
Is there a reason why no [JsonIgnore] attribute was added to this field?
The text was updated successfully, but these errors were encountered: