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>
Before this, a stream error would not be readable by user code, as the
`recv_features()` function would not even attempt to parse it. This
change allows application code to react to stream errors which are
received before stream features are received.
skip-changelog, because there's no release with the xmlstream module
yet.
This provides a clear and consistent interface for selecting TLS-related
features on both (xmpp and tokio-xmpp) crates. All feature checks have
been revamped. All working combinations have been tested (including a
connectivity test + --all-features docs build) using:
```
set -xeuo pipefail
features=(aws_lc_rs ring ktls,aws_lc_rs ktls,ring aws_lc_rs,ring aws_lc_rs,ring,ktls native-tls rustls-any-backend)
export RUSTFLAGS=" -D warnings"
cargo test --no-default-features
cargo test
for feature in ${features[@]}; do
echo ">>> BUILDING with $feature" 2>&1
# Running code or building examples cannot succeed with rustls-any-backend.
features="starttls,$feature"
if [ "$feature" != 'rustls-any-backend' ]; then
if ! cargo test --no-default-features --features="$features"; then
echo ">>> BUILD FAILED for tls feature set: $features" >&2
exit 1
fi
set +e
timeout -sINT -p -k 2 3 cargo run --no-default-features --features="$features" --example keep_connection -- test@hub.sotecware.net "$(pass xmpp/test@hub.sotecware.net)"
status="$?"
set -e
if [ $status -ne 0 ]; then
echo ">>> keep_connection did not shut down cleanly! (status: $status)" >&2
exit 1
fi
else
if ! cargo build --no-default-features --features="$feature"; then
echo ">>> BUILD FAILED for tls feature set: $features" >&2
exit 1
fi
fi
done
RUSTDOCFLAGS="--cfg docsrs" RUSTFLAGS="--cfg xmpprs_doc_build" cargo +nightly doc -Zrustdoc-map --all-features
```
For this, we had to:
- Make the choice(s) of backend explicit in tokio-xmpp features
- Avoid picking a backend through transitive dependencies in xmpp
(reqwest->hyper->rustls).
In addition, we choose the default rustls backend by default and adapt
the examples so that they can cope with either situation.
This way, we don't end up with breakage if rustls is built with more
than one provider enabled. This can (and does) happen if a transitive
dependency (such as reqwest *hinthint*) depends on a specific,
non-default rustls backend.
The user may have to call some global rustls functions depending on the
exact feature set of rustls enabled. Hence, we have to expose it.
Concretely, if the user somehow ends up with more than one rustls
backend, they have to configure one of the backends as default backend
at startup.
The stream may have read some data already (such as the stream
header). When initially writing this code, I had missed that we do
actually also carry the parser state over: I misread the
AsyncReader::wrap(..) command as something like AsyncReader::new, i.e.
starting off with fresh parser state, so I assumed that we need a fresh
XmlLangStack, too.
This is wrong: `p` comes from `self.parser.into_inner()` above and is
the parser state, so we need to carry the lang stack along, as well.
This fixes:
```
thread 'tokio-runtime-worker' panicked at […]/rxml-0.13.1/src/xml_lang.rs:87:13:
pop from empty XmlLangStack
```
happening during stream shutdown. The panic was first reported by
@ppjet6, so thanks for that and the keen eye.
skip-changelog, because the bug has not been released yet.
