Openfire

Openfire queries users for disco#info after each presence change (CAPS is being polled)

Details

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

Description

After a user changes its presence, Openfire queries the user for Service Discovery, every time.

One such query is expected: the CAPS verification string needs to be calculated / verified. Apparently, Openfire keeps asking for the corresponding features/identities for every VER string it gets.

Through debugging, I've noticed that the VER string that Openfire calculates based on the response to the disco#info request, does not match the VER string that was in the original presence that the user sent out. This happens for at least two clients: Pidgin and Gajim.

Is the verification calculation in Openfire correct?

Activity

Hide
Guenther Niess added a comment -

Hi Guus, with Pidgin, I can't reproduce this:

(10:13:41) jabber: Sending (ssl) (alice@domain.lt/Pidgin): 
<presence>
  <priority>1</priority>
  <c xmlns='http://jabber.org/protocol/caps' node='http://pidgin.im/' hash='sha-1' ver='AcN1/PEN8nq7AHD+9jpxMV4U6YM=' ext='voice-v1 camera-v1 video-v1'/>
  <x xmlns='vcard-temp:x:update'><photo/></x>
</presence>

and

(10:13:41) jabber: Recv (ssl)(304): 
<presence from="alice@domain.lt/Pidgin" to="bob@turing.domain.lt">
  <priority>1</priority>
  <c xmlns="http://jabber.org/protocol/caps" node="http://pidgin.im/" hash="sha-1" ver="AcN1/PEN8nq7AHD+9jpxMV4U6YM=" ext="voice-v1 camera-v1 video-v1"/>
  <x xmlns="vcard-temp:x:update"><photo/></x>
</presence>

Maybe I should look on this in more detail.

Show
Guenther Niess added a comment - Hi Guus, with Pidgin, I can't reproduce this:
(10:13:41) jabber: Sending (ssl) (alice@domain.lt/Pidgin): 
<presence>
  <priority>1</priority>
  <c xmlns='http://jabber.org/protocol/caps' node='http://pidgin.im/' hash='sha-1' ver='AcN1/PEN8nq7AHD+9jpxMV4U6YM=' ext='voice-v1 camera-v1 video-v1'/>
  <x xmlns='vcard-temp:x:update'><photo/></x>
</presence>
and
(10:13:41) jabber: Recv (ssl)(304): 
<presence from="alice@domain.lt/Pidgin" to="bob@turing.domain.lt">
  <priority>1</priority>
  <c xmlns="http://jabber.org/protocol/caps" node="http://pidgin.im/" hash="sha-1" ver="AcN1/PEN8nq7AHD+9jpxMV4U6YM=" ext="voice-v1 camera-v1 video-v1"/>
  <x xmlns="vcard-temp:x:update"><photo/></x>
</presence>
Maybe I should look on this in more detail.
Hide
Guus der Kinderen added a comment -

Guenther: the entity that sends the presence stanza (alice, in your example) will be queried by the server for disco#info (typically a few seconds after the stanza was sent). I can reproduce this in Pidgin.

I've fixed the issue locally. The hash calculation in Openfire didn't take all of the fields into account that it should. I will clean up the code and commit it later today.

Show
Guus der Kinderen added a comment - Guenther: the entity that sends the presence stanza (alice, in your example) will be queried by the server for disco#info (typically a few seconds after the stanza was sent). I can reproduce this in Pidgin. I've fixed the issue locally. The hash calculation in Openfire didn't take all of the fields into account that it should. I will clean up the code and commit it later today.
Hide
Guus der Kinderen added a comment -

I am removing the "Entity Capabilities Pending Hashes" cache. It is extremely unlikely that a response to an IQ query that has been send by a server node is received by another cluster node. This might occur if the original server node crashes after sending the request. If this does happen, another server node will simply ignore the result. The VER string won't be resolved during that attempt, but this doesn't impact users much. The VER string will be resolved after the next presence change.

The minute impact does not validate the rather large overhead of maintaining a clustered cache. It will be replaced with a local map.

Show
Guus der Kinderen added a comment - I am removing the "Entity Capabilities Pending Hashes" cache. It is extremely unlikely that a response to an IQ query that has been send by a server node is received by another cluster node. This might occur if the original server node crashes after sending the request. If this does happen, another server node will simply ignore the result. The VER string won't be resolved during that attempt, but this doesn't impact users much. The VER string will be resolved after the next presence change. The minute impact does not validate the rather large overhead of maintaining a clustered cache. It will be replaced with a local map.
Hide
Guenther Niess added a comment -

You are right, good finding

Show
Guenther Niess added a comment - You are right, good finding
Hide
Guus der Kinderen added a comment -

A new implementation of CAPS has been committed. I'll spend some more time to remove other clustered caches, as those are adding overhead rather than preventing resource usage too.

Show
Guus der Kinderen added a comment - A new implementation of CAPS has been committed. I'll spend some more time to remove other clustered caches, as those are adding overhead rather than preventing resource usage too.
Hide
Guus der Kinderen added a comment -

Actually, let's not. It'll force us to seriously review eviction policies. I'd rather not mess with that just before the upcoming release.

Show
Guus der Kinderen added a comment - Actually, let's not. It'll force us to seriously review eviction policies. I'd rather not mess with that just before the upcoming release.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: