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

Response to PING #629

Open
Pistacchione opened this issue Jan 22, 2019 · 11 comments
Open

Response to PING #629

Pistacchione opened this issue Jan 22, 2019 · 11 comments

Comments

@Pistacchione
Copy link

Hi,

I try to response a ping request, but every time the library sent this message:

<iq to="fakedomain" from="user1@fakedomain/491531e" id="89FD2BFFB1DBCB38" type="error" xmlns="jabber:client">
  <ping xmlns="urn:xmpp:ping"/>
  <error type="cancel">
    <service-unavailable xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/>
  </error>
</iq>

How can I manage the ping?

@Pistacchione
Copy link
Author

After some trial and error I manage the ping using iq package in this way:

iqCallee.get('urn:xmpp:ping', 'ping', ctx => { return {} })

but I don't known for sure if this is the right way to manage ping request

@sonnyp
Copy link
Member

sonnyp commented Jan 29, 2019

Please note that an error reply is a correct ping response as it fulfils its purpose.

iqCallee.get('urn:xmpp:ping', 'ping', ctx => { return {} }) is correct, it replies with an empty response

Is there any specific reason why answering with an error wasn't enough? I can imagine enabling ping namespace response in @xmpp/client.

@Pistacchione
Copy link
Author

Hi @sonnyp

I'm not an expert of XMPP but based on ping documentation https://xmpp.org/extensions/xep-0199.html if I received a ping message:

<iq from='capulet.lit' to='[email protected]/balcony' id='s2c1' type='get'>
  <ping xmlns='urn:xmpp:ping'/>
</iq>

I must return a response like this:

<iq from='[email protected]/balcony' to='capulet.lit' id='s2c1' type='result'/>

I must return an error only if I don't support ping namespace:

<iq from='[email protected]/balcony' to='capulet.lit' id='s2c1' type='error'>
  <ping xmlns='urn:xmpp:ping'/>
  <error type='cancel'>
    <service-unavailable xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/>
  </error>
</iq>

Correct me if I say something wrong :)
Thanks

@sonnyp
Copy link
Member

sonnyp commented Jan 29, 2019

@Pistacchione you're right, my point is that answering a ping request with an error is considered a ping response anyway because it fulfils the purpose of the ping.

Anyway, I will add ping response to @xmpp/client to remove the ambiguity

@dlameri
Copy link

dlameri commented May 14, 2019

This ping response as an error keeps happening, right?

@IProudNoob
Copy link

This ping response as an error keeps happening, right?

I tested this problem. It's bug is did not fixed. After some time to be disconnected.

@gabsoftware
Copy link

Was the ping response added to @xmpp/client?

@netmikey
Copy link
Contributor

Up until v0.13.0, xmpp.js seems to respond with the error as discussed above.

I put in place a ping-response implementation:

client.on('stanza', (stanza) => {
  // XEP-0199: XMPP Ping
  if (stanza.is('iq') && stanza.attrs.type === 'get' && stanza.getChild('ping', 'urn:xmpp:ping')) {
    client.send(
      xml("iq", { from: stanza.attrs.to, to: stanza.attrs.from, id: stanza.attrs.id, type: 'result' })
    );
  }
});

This results in double response to the same ping:

IN
<iq type="get" id="792-276" from="my-server" to="2cwdh9zdl8@my-server/2cwdh9zdl8"><ping xmlns="urn:xmpp:ping"/></iq>
OUT
<iq from="2cwdh9zdl8@my-server/2cwdh9zdl8" to="my-server" id="792-276" type="result"/>
OUT
<iq to="my-server" from="2cwdh9zdl8@my-server/2cwdh9zdl8" id="792-276" type="error"><ping xmlns="urn:xmpp:ping"/><error type="cancel"><service-unavailable xmlns="urn:ietf:params:xml:ns:xmpp-stanzas"/></error></iq>

While this seems to be accepted by my XMPP Server (Openfire), it surely isn't pretty.

@sonnyp
Copy link
Member

sonnyp commented Aug 28, 2021

@netmikey this is not how you're supposed to reply to iqs with xmpp.js. The solution is in #629 (comment)

See https://github.com/xmppjs/xmpp.js/tree/main/packages/iq#callee

@netmikey
Copy link
Contributor

Thanks for correcting me, @sonnyp! I had looked at the @xmpp/client documentation but had overseen the fact that iq handling is (understandably) documented in the iq package.

@iravan
Copy link

iravan commented Nov 3, 2023

@netmikey this is not how you're supposed to reply to iqs with xmpp.js. The solution is in #629 (comment)

See https://github.com/xmppjs/xmpp.js/tree/main/packages/iq#callee

Thanks. I think it should be prebuilt/by-default since it's in the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants