sombhardwaj / asmack

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

RosterPacketListener handles RosterPacket.ItemType.remove incorrectly #15

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What are you doing to produce the error?
1. Server sends subscription remove packets during getRoster
2. getRoster fails
3.

What is the expected output?

getRoster completes successfully

What do you see instead? (Please attach a debug enabled logcat)

getRoster fails

What version of aSmack / Android / Device do you use?

all versions

What server do you use? Is there a public server to reproduce the problem?

chat.facebook.com

What else might help us to reproduce and hunt down the problem?

Roster.class from asmack is different from smack 3.1.0 - the Roster.class
from 3.1.0 looks like it handles remove requests differently from the
asmack Roster.class

MORE INFO
----------
http://xmpp.org/extensions/xep-0147.html#actions-roster-remove (3.2.2
Remove Roster Item)

Inital remove requests from getRoster
--------------------------------------

<iq from="johnny@c"
to="johnny@c/Smack_fabc92c4_4866950E4C091"
id="fbiq4866950F2E6E2" type="set"><query xmlns="jabber:iq:roster"><item
jid="u1287931292@c" subscription="remove"/></query></iq><iq
from="johnny@c"
to="johnny@c/Smack_fabc92c4_4866950E4C091"
id="fbiq4866950F2E71F" type="set"><query xmlns="jabber:iq:roster"><item
jid="u663073257@c" subscription="remove"/></query></iq><iq
from="johnny@c"
to="johnny@c/Smack_fabc92c4_4866950E4C091"
id="fbiq4866950F2E744" type="set"><query xmlns="jabber:iq:roster"><item
jid="u701803063@c" subscription="remove"/></query></iq>

Original issue reported on code.google.com by zenp...@gmail.com on 14 May 2010 at 9:15

GoogleCodeExporter commented 8 years ago
Investigating further, I think the problem is this:

1) Facebook is sending subscription remove packets with Roster
2) Facebook is not sending Roster Version (Facebook Bug)

3) As NO version, persistentStorage is being set to null:

if(rosterPacket.getVersion()==null){
   persistentStorage=null;
} else{
   version = rosterPacket.getVersion();
}

4) As persistentStorage is now NULL, remove packets not handled:

if(persistentStorage!=null){
   for (RosterPacket.Item i : rosterPacket.getRosterItems()){
      if(i.getItemType().equals(RosterPacket.ItemType.remove)){
         persistentStorage.removeEntry(i.getUser());
      } else{
         persistentStorage.addEntry(i, version);
      }
   }
}

Does anyone have a workaround for this ?

Any help would be grateful.

Best

Original comment by zenp...@gmail.com on 14 May 2010 at 11:03

GoogleCodeExporter commented 8 years ago
Sorry about that, the roster versioning was pulled from till glocke as it 
helped.

I've cross-posted on http://github.com/dereulenspiegel/smack/issues/issue/1 and 
will
try to get a fix in a reasonable timeframe, hopefully till next week.

Original comment by rtreffer@gmail.com on 14 May 2010 at 6:15

GoogleCodeExporter commented 8 years ago
Thanks rtreffer,

Unfortunately my java skills are not that advanced, but am happy to help where 
I can.

Out of interest, what are you using to compile the source into a jar file? I am
trying to learn as I go :)

Best

Johnny

Original comment by zenp...@gmail.com on 15 May 2010 at 11:54

GoogleCodeExporter commented 8 years ago
Compiling is done with the bash script at http://github.com/rtreffer/asmack -
http://github.com/rtreffer/asmack/blob/master/build.bash

It's rather complicated because it needs a linux environment to build.

Regards,
  RT

Original comment by rtreffer@gmail.com on 15 May 2010 at 1:02

GoogleCodeExporter commented 8 years ago
Sorry, for the bug that I introduced. I tested roster versioning also against
Openfire and eJabberd without RosterVersioning and it seemed to work. I will 
fix this
tomorrow.

Original comment by till.klo...@gmail.com on 15 May 2010 at 3:48

GoogleCodeExporter commented 8 years ago
No need to apologize, roster versioning is a well needed feature, and a fix that
comes tomorrow is more than I could expect :-)

@zenpods: I'll cook a new version with that fix and would be happy if you could 
test
it afterwards....

Original comment by rtreffer@gmail.com on 15 May 2010 at 8:22

GoogleCodeExporter commented 8 years ago
Wow, many thanks to you both for your help so promptly.

Original comment by zenp...@gmail.com on 15 May 2010 at 8:28

GoogleCodeExporter commented 8 years ago
Please try 
http://code.google.com/p/asmack/downloads/detail?name=asmack-issue15.jar

Original comment by rtreffer@gmail.com on 18 May 2010 at 2:57

GoogleCodeExporter commented 8 years ago
zenpods, can you clarify why you think Facebook has a bug by not sending the the
Roster Version?  The server simply doesn't support XEP-0237.  The fact that the
server doesn't tolerate "ver" attributes from the client is tracked separately 
at
<http://bugs.developers.facebook.com/show_bug.cgi?id=8744> and should be fixed 
soon.

Original comment by dre...@gmail.com on 18 May 2010 at 4:59

GoogleCodeExporter commented 8 years ago
thanks for the update dreiss. It's definetly not a fb bug. I've uploaded an
experimental fix and hope to have this sorted out soon.

Regards,
  Rene

Original comment by rtreffer@gmail.com on 18 May 2010 at 6:09

GoogleCodeExporter commented 8 years ago
Hi there

Thanks for the upload.

asmack-issue15.jar is still causing getRoster to fail at chat.facebook.com. I 
think
it still is not sure how to handle these remove subscriptions?

Is there anything I can do to help?

dreiss, yes your right, the bugzilla post you mentioned, I mis-understood it. I
thought it was related this problem.

Best regards

Johnny

Original comment by zenp...@gmail.com on 19 May 2010 at 10:26

GoogleCodeExporter commented 8 years ago
Ok, here is how you might help:

- Enable full protocol dump (with setDebug(true))
- Run adb logcat and dump to a file (adb logcat > dump)
- Remove all <sasl> parts (login credentials)

TIA,
  Rene

Original comment by rtreffer@gmail.com on 20 May 2010 at 11:51

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Hi,

I have put dump into pastebin, couldnt work out if it should be formatted.

http://pastebin.com/aXkCi3cC

Line 27 - This is the point where getRoster returns null. I think because it 
doesnt
know what to do with the FIRST remove subscription packet.

Original comment by zenp...@gmail.com on 21 May 2010 at 11:28

GoogleCodeExporter commented 8 years ago
Could I also 'second' this issue. 

I have attempted to make a fix for it but am unable to get the build script to 
work.

Let me know if I can help in anyway.
Thankyou,
Harry

Original comment by wildbeat...@googlemail.com on 30 May 2010 at 12:50

GoogleCodeExporter commented 8 years ago
Hi,

Same as 'wildbeatcommunication', let me know if I can help somehow.

Thank you all,
Oskar

Original comment by koki...@gmail.com on 1 Jun 2010 at 12:54

GoogleCodeExporter commented 8 years ago
nothing yet? nothing there? impossible?

Original comment by koki...@gmail.com on 11 Jun 2010 at 9:03

GoogleCodeExporter commented 8 years ago
This issue has resolved automatically, as Facebook have now fixed roster 
versioning:

http://bugs.developers.facebook.com/show_bug.cgi?id=8744

Many thanks for your support.

Best wishes.

Original comment by zenp...@gmail.com on 1 Jul 2010 at 9:29

GoogleCodeExporter commented 8 years ago
asmack-issue15 do not support multiUserChat?

Original comment by liu0198...@gmail.com on 6 Jun 2011 at 2:59