The car can’t go offroad every time someone painted a swear word on the road or painted a deer differently on the sign near the road.
If some instruction that my client doesn’t support anyways comes in, I am better off ignoring it than disconnecting, imo.
By disconnecting, you don’t inform the user of what happened nor did you prevent it from happening again, nor can the user do anything about it.
End-user experience suffers TRAMENDOUSLY!
If you want to create a callback that says “send errors here” - fine. But logging out the client implies a non-recoverable error which doesn’t allow the session to continue and therefor REQUIRES a new session.
I do not see “delay” instruction being mission-critical to the login process.
If you do - I’d love to hear how/why. (I don’t know the exact use for it, so merely trying to understand).
It was Octro and they said they’ll fix it (in time).
But not fixing this bug means that any company using Smack for a jabber or gtalk can be taken down in 1 shot by 1 user (malicious or not) for indeterminate amount of time.
And if they find it was a known problem - it will be the last time ignite software is taken seriously in a corporate environment.
This is surely a security problem that would deem OpenFire or Smack unusable in any corporate environment. I think it’s pretty clear. Hope you guys see this.
I agree that having a failed login due to this is unacceptable, so it will be fixed, hence the Jira task.
Unless I am mistaken though, Octro should not be putting this element in at all. From my understanding, this extension is only meant to be inserted by the server on packets that are stored for later delivery, such as it is used in MUC and pubsub. The purpose of the delay element is to indicate when the packet was originally sent, thus it is basically a server timestamp.
That being said, there is nothing that requires the server to remove it either if it is sent by the client.
On a related note, if they are going to fix it, they may also want to update to the Delayed Delivery spec, as the one they are currently using (Legacy Delayed Delivery) is, well, legacy and obsolete.
Sounds good. I did confirm as well that this is meant to be inserted by the server or a component. It is not meant to be sent from a client, which I believe is what Octra is.
Do I undestand your last comment on SMACK-390 correct? There will be no multiple providers with different behaviors? If so, I would suggest a SmackConfiguration option that, when enabled, still throw the exception back the application when parsing extensions in presence stanzas. I even would it enable it as default.
I see myself in the near feature debugging an obscure smack problem, only to find out the reason was a catched and relatively silent loged exception… That’s one of the reasons I don’t like the fix for SMACK-390 as it is at the moment.
I may still put in multiple providers, so the parsing can be strict or lenient, but that is a now a different issue than the presence extension.
Due to the login issue, I thought it would be best to make the presence parsing more robust by simply omitting the extension if, for any reason, it is well formed but unparseable. I think this makes more sense anyway than losing presence packets because a custom extension was added that causes an exception on the receiver. It is also reasonably consistent with how other attributes (priority and show) are already handled in the presence parser, in that errors are logged and if possible default values are inserted.
A richer logging mechanism would be nice as well, but I don’t foresee that happening any time soon either, due to it causing unwanted dependencies in the base library.
That’s one of the reasons I don’t like the fix for SMACK-390 as it is at the moment.