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.
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.
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.
It could have been resumed without going into hibernation first, i.e.
when the client notices the disconnect before the server, or if it
switches networks etc.
Still having the connection on the session may cause unintentional
behavior, such as the session being treated as if connected, even tho
the connection has been closed.
To ensure that if a session is replaced after it has gone into
hibernation, it does not come back and cause trouble for the new session
(see previous commit).
The resumption_token is removed when the session is closed via the
pre-session-close event, signaling that it cannot be resumed, and
therefore no hibernation timeout logic should be invoked.
Fixes that if a session somehow is replaced by a new one using the same
resource (which is the common behavior), the old session would still be
around until it times out at which point it sends `<presence
type="unavailable"/>` which would look as if it came from the new
session, ie appearing offline to everyone including MUCs.
This overloads that flag a bit, but it has the intended effect of
stopping outgoing_stanza_filter() from queueing stanzas.
Fixes a traceback because of the queue having been removed somewhere
around here, since it is no longer needed.
Thanks Martin for reporting
This brings back the queue size limit that was once added, then removed
because destroying the session when reaching the limit was not great.
Instead, the queue wraps and overwrites the oldest unacked stanza on the
assumption that it will probably be acked anyway and thus does not need
to be delivered. If those discarded stanzas turn out to be needed on
resumption then the resumption fails.
All that was a complicated way to limit the number of resumable
sessions. Let's control resource usage some other way. This leaves the
essence of mapping resumption tokens to live sessions.
This keeps resumption state across reloads.
This allows clients that try to resume a session after a server restart
to at least know which of their pending outgoing stanzas were received
and which need to be re-sent.
This removes the limit on how many of those counters are kept, which
should be fixed eventually.
I have no idea what is going on in this code, which session is which?
Something has one of the sessions as an upvalue which is where the
filter checks for it.
As per a86ae74da96c the 'session' object here is the wrong session, so
the attempt to block stanzas from being added to the queue twice did not
work causing something of a leak.
Instead we have a leak of the previous session.