Smack

Smack 3.1.0 creates a new chat for every incoming message

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 3.1.0
  • Fix Version/s: 3.2.0
  • Component/s: Core
  • Labels:
    None
  • Acceptance Test - Add?:
    No

Description

Smack 3.1.0 creates a new chat for every incoming message

Forum Post Quote:
I've found a bug which took a few hours to figure out:

If the remote client doesn't supply a thread-id (psi doesn't), then Smack will create a new Chat for every thread. It can't ever reuse chats from ChatManager.jidChats because of an impedance mismatch.

The problem is, it puts raw jabber addresses as keys (foo@bar.org/Client), but gets jabber addresses cleaned by StringUtils.parseBareAddress (foo@bar.org)

FIX is described here:
http://www.igniterealtime.org/community/message/189614

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
ankit nitin added a comment -

Major problem
--------------------

ThreadID is everywhere documented as the unique identifier for chat messages. If a packet do not contain a threadID then how smack figure out the chat object to which the packet needs to be delivered ?

Show
ankit nitin added a comment - Major problem -------------------- ThreadID is everywhere documented as the unique identifier for chat messages. If a packet do not contain a threadID then how smack figure out the chat object to which the packet needs to be delivered ?
Hide
Permalink
Konrad Mrozek added a comment -

Hi, I think the patch which was given is sufficient. It corrects the bug. Why isn't it commited to trunk?

ThreadID is a problem for e.g. Psi or iChat. These programs don't attach threadid to message "chat" packet at all. So someone (as I have seen in SVN called gato) added workaround to this bug of clients. But he probably didn't think about resources attached address or didn't change line 177 to

String userJID = StringUtils.parseBareAddress(message.getFrom());

I think, that patch from ilja makes assigning chats without threadid more accurate.

So I don't know why it's not commited yet.

Show
Konrad Mrozek added a comment - Hi, I think the patch which was given is sufficient. It corrects the bug. Why isn't it commited to trunk? ThreadID is a problem for e.g. Psi or iChat. These programs don't attach threadid to message "chat" packet at all. So someone (as I have seen in SVN called gato) added workaround to this bug of clients. But he probably didn't think about resources attached address or didn't change line 177 to String userJID = StringUtils.parseBareAddress(message.getFrom()); I think, that patch from ilja makes assigning chats without threadid more accurate. So I don't know why it's not commited yet.
Hide
Permalink
Guus der Kinderen added a comment -

The unpatched source matches the bare JIDs against a map that is most likely to hold full JIDs. I've applied the patch as provided in the community discussion.

Show
Guus der Kinderen added a comment - The unpatched source matches the bare JIDs against a map that is most likely to hold full JIDs. I've applied the patch as provided in the community discussion.
Hide
Permalink
Guus der Kinderen added a comment -

Reopened: evaluate http://www.igniterealtime.org/community/message/201120#201120

Show
Guus der Kinderen added a comment - Reopened: evaluate http://www.igniterealtime.org/community/message/201120#201120
Hide
Permalink
Robin Collier added a comment -

Added tracking of chats by base jid (only full jid currently). Now when a chat message with no thread id is received, a match is attempted by both full and base jid, thus insuring a match if a chat to that user has already been created. Also made the lookups threadsafe.

On a different note though, I noticed that it also attempts to do this same match when there is a thread ID as well but no existing chat exists for that ID. I would think that should always result in a new chat, not matching a chat with a different id but the same JID, which is what it does now.

Show
Robin Collier added a comment - Added tracking of chats by base jid (only full jid currently). Now when a chat message with no thread id is received, a match is attempted by both full and base jid, thus insuring a match if a chat to that user has already been created. Also made the lookups threadsafe. On a different note though, I noticed that it also attempts to do this same match when there is a thread ID as well but no existing chat exists for that ID. I would think that should always result in a new chat, not matching a chat with a different id but the same JID, which is what it does now.

People

  • Assignee:
    Robin Collier
    Reporter:
    Walter Ebeling
Vote (5)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved: