Prosody would let the stream opening timeout if it receives it in more
than one TCP packet, which seems like a bug.
The behaviour was highly non-deterministic, a release build would almost
always (but not always) succeed to connect to a localhost Prosody, a
debug build would almost always (but not always) fail, and
RUST_LOG=trace would make any build fail to connect, as would strace.
My theory is that successful runs rely on the kernel merging all five
TCP packets in a single one, and any tracing would add sufficient delay
in-between to let it send every packet on its own.
This introduces the FallibleStreamElement type which, instead of
failing parsing altogether, captures certain types of parse errors and
allows the user (in this case, the stanzastream) to react to these
errors appropriately.
This allows us to drop invalid stanzas (e.g. with malformed values in
strictly-checked fields) instead of failing the entire stream.
Fixes#172.
While the FromXml bound is necessary for the key implementation of the
XmlStream (the `impl Stream`), the AsXml bound is not as intrinsic: the
Sink implementation can take any AsXml implementation on each separate
send invocation.
By removing this bound, we can add extra logic *only* to the parsing,
without having to worry about effects on the sending side.
Concretely, we'll be able to create a wrapper type for XmppStreamElement
which allows us to capture errors during parsing, without having to
worry about what happens during serialisation.
Adds an option to split the `tokio-xmpp::Client` into a Sender and
Receiver. To enable this, a client worker is added which drives the
stream in the background.
Up to now, the xmpp-rs projects have been very strict about incoming
data. This has served us, as developers of the libraries, well,
uncovering bugs in our and remote implementations which we could then
get fixed.
However, this behaviour is unexpected to users of the library. In the
XMPP world, unexpected child elements and attributes are generally
expected to be ignored. While this could be opted-into previously, the
feature flag for that sounded more dangerous than it was
("disable-validation"). In addition, the tribal knowledge needed to know
about that feature flag may not have reached some people who tried the
library and gave up because of that.
With this change, we make the non-pedantic behaviour the default. For
development and debugging purposes, users can always opt into the
pedantic behaviour as needed, using the newly-introduced `pedantic`
feature flags on all affected crates.
Features must never be duplicated, so we can use a BTreeSet instead of a
Vec to be nicer for users.
It makes the internal API for computing caps and ecaps2 a bit worse,
because it was expecting a slice directly, so for now let’s collect the
BTreeSet into a Vec when computing the thing. A refactor to use
Iterator might make it better eventually, but I won’t work on that
before profiling it.
This was already just a wrapper around a Vec<String>, but we can do away
with the wrapper thanks to #[xml(extract)].
I’ve also replaced Vec with BTreeSet, since that corresponds better to
how the mechanisms are.
From the prosody@ room:
- 0114 doesn't mention the removal of @version on the stream, and it
refers to 3920 which has it as a MUST.
- 0114 streams have historically never used @version="1.0"
- @version="1.0" implies stream features which 0114 doesn't have.
- There was a proposal years ago to fix this but a new XEP was preferred
(0225).
The change here uses a compile-time check, and may have to change when
support for 0225 arrives if it's still gated behind the "component"
feature (even though it may be weird to have both under the same
feature). We'll see when we get there.
Signed-off-by: pep <pep@bouah.net>
