adrianchia / javapns

Automatically exported from code.google.com/p/javapns
0 stars 1 forks source link

Payload should throw an exception when adding content that causes it to exceed the size limit #74

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Actually a question rather than an issue: if the payload size is over 256, does 
javapns shrink it before sending to apple gateway?

Thanks.

Jim 

Original issue reported on code.google.com by wguo1...@gmail.com on 12 Oct 2011 at 9:35

GoogleCodeExporter commented 9 years ago
I'm not sure how the library could attempt to shrink a payload...  how would it 
know where to cut in your data?

If the payload exceeds 256 bytes, an exception is thrown.  See:
http://code.google.com/p/javapns/source/browse/trunk/src/javapns/notification/Pa
yload.java#112

Original comment by sype...@gmail.com on 12 Oct 2011 at 9:52

GoogleCodeExporter commented 9 years ago
I should say cut the alert body. I saw an example from 
https://github.com/notnoop/java-apns/blob/master/src/main/java/com/notnoop/apns/
PayloadBuilder.java

It would be nice to have this feature in the lib, but it appears tricky to 
calculate length and shrink the body. I will do some experiments to see how it 
works.

Thanks.

Jim 

Original comment by wguo1...@gmail.com on 13 Oct 2011 at 1:57

GoogleCodeExporter commented 9 years ago
I'm not sure I would want a library to start changing my own data.  Personally, 
I think that this should be left to the developer to adjust his data to fit the 
requirements.  But I guess the library could be improved in when it reports 
that the payload is oversize...  For now it will throw the exception only when 
compiling the payload ... not when building it.  It would be better if the 
exception was thrown whenever adding some data to the payload causes it to 
exceed the maximum size allowed..  I will update this issue to reflect the 
enhancement request we are formulating here :)

Original comment by sype...@gmail.com on 13 Oct 2011 at 8:27

GoogleCodeExporter commented 9 years ago
Thanks. I actually have trouble to shrink the body. If the whole payload is 
over 256, I will shrink the alert body. The trouble is that I still did not 
receive the notification even though the whole payload size is 256 exactly. The 
total number of bytes (including command, device token, payload len etc) sent 
out is 293.

Playing more ... 

Original comment by wguo1...@gmail.com on 13 Oct 2011 at 9:14

GoogleCodeExporter commented 9 years ago
Indeed, as Apple'a doc says, the payload part of the message cannot be longer 
than 256 bytes, and consequently the maximum message size is 293 bytes exactly.

Are you able to send payloads smaller than 256 bytes?  In other words, are you 
sure it is the size limit that is the problem here?

Original comment by sype...@gmail.com on 13 Oct 2011 at 9:54

GoogleCodeExporter commented 9 years ago
I discovered that there was a logic error in the library (from older code I 
assumed was correct when I came onboard) at the place where the payload's size 
is written to the output stream;  the payload's size was not being correctly 
converted to individual bytes if it was larger than 255 (ie taking more than 
one byte to write).  A payload of 256 bytes would cause the library to write 
0x00 and 0x00 as the two bytes representing the payload size, instead of 0x01 
and 0x00.  Consequently, trying to send a payload of exactly 256 bytes would 
return an error-response packet indicating that a payload is missing :)  Had 
you checked the error response after your tests with 256 bytes?

With the changes I just made to the library, I'm now able to push 256-bytes 
payloads successfully to my iPhone.  If I try to push a 257-bytes payload (I 
had to disable the library's size checking just to test this :), I 
appropriately get an error-response from Apple with the "Invalid payload size" 
error, which is what I would expect.

I have also added a mechanism to the Payload class which allows it to estimate 
its future size if a specific property is added to it, and then an option (off 
by default) which will reject any new property which causes the size estimate 
to exceed the maximum size allowed.  When the option is turned on for a blank 
payload and that I try to add a very large alert, the addAlert method 
automatically throws an exception now.  This feature is off by default for now, 
so it must be turned on for each payload if desired.

I have committed everything to the trunk, so if you would like to try this, you 
can download the JavaPNS library from SVN (see Download wiki page for nightly 
builds).

Original comment by sype...@gmail.com on 14 Oct 2011 at 1:48

GoogleCodeExporter commented 9 years ago
Great! It works now. Thanks for the quick fix.

Jim 

Original comment by wguo1...@gmail.com on 14 Oct 2011 at 1:58

GoogleCodeExporter commented 9 years ago
Thank you for the feedback!

Closing as Fixed.

Original comment by sype...@gmail.com on 14 Oct 2011 at 3:06