Order of events when connecting

Hi, I’m currently doing some work on XEP-0198, and I’m encountering different issues with the SMACK integration.

Today, I checked the order of events when logging in (for session resumption, this needs to be: STARTTLS, auth, compression, resume or bind).

While looking at the code, I found several issues:

  1. compression is negotiated after binding (compression restarts the , and therefore violates http://xmpp.org/rfcs/rfc6120.html#bind-rules-restart)
  2. there is no way to insert a callback between auth and binding
  3. binding is done in SASLAuthentication.bindResourceAndEstablishSession(), even though it does not have anything in common with authentication.

The latter (#3) is probably due to the fact that SMACK falls back to NonSASLAuthentication, which does both auth and binding in a single step.

Now, I would suggest the following refactorings:

  1. completely remove NonSASLAuthentication and all related code (http://xmpp.org/extensions/xep-0078.html is deprecated since 2008!)
  2. move the binding code to Connection or XMPPConnection/BOSHConnection, allowing for callbacks/filters (like compression and XEP-0198) to be inserted between auth and binding.

I am willing to provide patches for both, if they have a chance to land in the official repo.

P.S: I encounter many small and middle-scale issues (like typos) in the code, and I would be grateful if I could directly create issues in the tracker. So far, Florian (flowdalic) is doing a great job in helping me out, but bugging him with every single case is rather cumbersome.

I think it is a good idea to remove the non-SASL code and therefore reduce the complexity of Smack core because of the usual advantages:

  • less code that could contain errors
  • improved code readablity
  • better maintainability

And please allow Ge0rg to create issues. He is discovering a lot of minor and major (e.g. race conditions) issues in Smack while working on XEP-0198 support for Smack.

Patches are good, make sure they build against trunk, as these changes will have to be against a future major release. Just attach them to the appropriate JIRA task.

I am assuming Flow put in a request on your bahalf to grant you JIRA access.

rcollier wrote:

Patches are good, make sure they build against trunk, as these changes will have to be against a future major release. Just attach them to the appropriate JIRA task.

What’s your opinion about removing the non-SASL code?

rcollier wrote:

I am assuming Flow put in a request on your bahalf to grant you JIRA access.

No, Georg asked for the ability to create issues in the last paragraph of his post.

Since it is an obsolete spec, I think we should remove it in trunk, which would effectively remove it in the next major release.

I will ask Daryl to give Georg Jira access.

SMACK-446 - Remove non-SASL authentication code