Skip to content
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

Problem with onRosterPush function when receiving a push from server #493

Closed
teseo opened this issue Sep 30, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@teseo
Copy link
Contributor

commented Sep 30, 2015

Hello,

I am experimenting that when receiving a roster push from the server the roster list is not updated. Debugging I found this piece of code in the RosterContacts collection within converse.js file:

            onRosterPush: function (iq) {
                /* Handle roster updates from the XMPP server.
                 * See: https://xmpp.org/rfcs/rfc6121.html#roster-syntax-actions-push
                 *
                 * Parameters:
                 *    (XMLElement) IQ - The IQ stanza received from the XMPP server.
                 */
                var id = iq.getAttribute('id');
                var from = iq.getAttribute('from');
                if (from && from !== "" && from != converse.bare_jid) {
                    // Receiving client MUST ignore stanza unless it has no from or from = user's bare JID.
                    converse.connection.send(
                        $iq({type: 'error', id: id, from: converse.connection.jid})
                            .c('error', {'type': 'cancel'})
                            .c('service-unavailable', {'xmlns': Strophe.NS.ROSTER })
                    );
                    return true;
                }
                converse.connection.send($iq({type: 'result', id: id, from: converse.connection.jid}));
                $(iq).children('query').find('item').each(function (idx, item) {
                    this.updateContact(item);
                }.bind(this));
                return true;
            },

My stanza contains in the from attribute actually the value of converse.connection.jid (user@host/resource) which is what it is used when sending the error stanza as well as when sending the result stanza back to server in this very function. converse.bare_jid is actually not used within that function at all other than in that if statement. If I manually change converse.bare_jid for converse.connection.jid in the if statement the roster list gets updated perfectly for both removing and adding.

Is converse.bare_jid supposed to be in that if ? if so, is there a clean way to make it work ?

Many Thanks!

jcbrand added a commit that referenced this issue Sep 30, 2015

@jcbrand

This comment has been minimized.

Copy link
Member

commented Sep 30, 2015

Hi @teseo, thanks for the report and explanation. Please see the fix I pushed 2 minutes ago.

@jcbrand jcbrand closed this Sep 30, 2015

@teseo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 1, 2015

@jcbrand lekker, dankie!

@jcbrand

This comment has been minimized.

Copy link
Member

commented Oct 1, 2015

:D

@teseo

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2015

Hello Jc,

I just updated to the last version and I am afraid the fix you pushed is not working as expected. I take my blame as I trusted it without testing it. I'm sorry for that.

Your fix is actually always allowing to enter in the mentioned if.

This is the code that actually works for me and I think it should be the fix:

javascript
if (from && from !== "" && from != converse.connection.jid) {
// Receiving client MUST ignore stanza unless it has no from or from = user's bare JID.
converse.connection.send(
$iq({type: 'error', id: id, from: converse.connection.jid})
.c('error', {'type': 'cancel'})
.c('service-unavailable', {'xmlns': Strophe.NS.ROSTER })
);
return true;
}


Again sorry for assuming instead of proper testing. If you give me green light, I'll push a pull request with this fix.

Dankie!
@jcbrand

This comment has been minimized.

Copy link
Member

commented Oct 19, 2015

Hi @teseo, don't worry about it. Please make a pull request with the new fix.

jcbrand added a commit that referenced this issue Nov 4, 2015

updates #493
Compare bare JID of from attr in roster update. @teseo's fix was too specific
and tailored for a misbehaving server (servers shouldn't be sending from the
full JID in a roster push).
@m0cs

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2015

Hi! rosterPush is working as expected? Im using punjab, but on roster update push function goes on but not update..

@jcbrand

This comment has been minimized.

Copy link
Member

commented Nov 12, 2015

Yes, I tested roster push before releasing 0.10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.