Prevents ASYNC-01 due to storage interactions in a timer.
Also considered modifying mod_c2s to allow passing arbitrary closures
into its runner thread but this seems like a big step away from the
current code for just this module.
Also considered creating a dedicated runner in mod_smacks, but ensuring
continuity across module reloads might be tricky.
We could further improve this in the next major version.
Given that there are recommendations floating around recommending 24
hours session lifetime, having buckets up to 10 minutes wouldn't be
useful in that case.
Would be nice if we had some way to automatically assign suitable number
series for buckets, scaled to what the configuration might be.
There shouldn't be one here but if there is, for some reason, it's
better to close it than have it around to wake up and possibly try to
destroy the session.
Since resumption is not supported on s2s currently, there is no point in
allocating resumption tokens. The code that removes entries from
session_registry is only invoked for c2s sessions, thus enabling
resumable smacks on s2s adds an entry that never goes away.
Sending stanzas with a remote session as origin when the stanzas have a
local JID in the from attribute trips validation in core.stanza_router,
leading to warnings:
> Received a stanza claiming to be from remote.example, over a stream authed for localhost.example
Using module:send() uses the local host as origin, which is fine here.
Fixes#1758
Introduced in 1ea01660c79a
In e62025f949f9 to and from was inverted since it changed from acting on
a reply to acting on the original stanza (or a clone thereof)
Unsure of the purpose of this check, you don't usually send stanzas to
your own full JID. Perhaps guarding against routing loops?
The check was present in the original commit of mod_smacks,
prosody-modules rev 9a7671720dec
Fixes#1757
These places seem to have been left since e62025f949f9
The logic around expected_h in should_ack() misbehaved, always comparing
with 0 + unacked instead of acked + unacked.
Many of these fall into a few categories:
- util.cache size, must be >= 1
- byte or item counts that logically can't be negative
- port numbers that should be in 1..0xffff
Conversations 2.10.10 and earlier expect this to be literally 'true' and don't
recognise '1'. This leads to it not attempting resumption with Prosody at all
since this change was introduced in 36ba170c4fd0.
Thanks to Zash for noticing, debugging and diagnosing this issue.
This issue is fixed in Conversations commit 052c58f3 (unreleased at the time
of writing).
The use of the error helpers creates an `<error/>` child element
containing the error condition. This is however not allowed as per
XEP-0198, which specifies that the error condition is to be a direct
child of the `<failed/>` stream management element.
This has triggered a fun reconnect loop in aioxmpp where it was
reported by a user [1].
[1]: https://github.com/horazont/aioxmpp/issues/382
Setting the .smacks field enables code paths that expects the queue to
be present. The queue is initialized in wrap_session_out(). With
opportunistic writes enabled this happens immediately on .sends2s(), so
the sending <enable> must happen before OR after these two lines, not in
the middle.
Using a timer was a hack to get around that stream features are not
available at the right time and sendq stanzas were stored as strings
so could not be counted properly. The later has now been fixed and the
former is fixed by recording the relevant stream feature on the session
so that the correct version of XEP-0198 can be activated once the
connection has been authenticated and is ready to start.
Doing this when creating a whole new session seems reasonable because it
is already expensive and this is when it may be adding to the old
session store, while a successful resumption should be plus-minus zero.
UUID seems like insane overkill for something user-scoped and not
security-sensitive. All that is needed is to avoid conflicts among what
should be relatively long-lived sessions.
