Compare commits

...

7 Commits

Author SHA1 Message Date
Jonas Schäfer 8a7a03f7a7 Merge branch 'feature/ignore-missing-disco-info' into 'main'
Ignore missing disco#info feature in disco#info responses

See merge request xmpp-rs/xmpp-rs!302
2024-04-17 07:55:43 +00:00
Jonas Schäfer 1293e9a3eb jid: fix incorrect type on Jid::Full
This does not matter much because users need to replace usages of these
anyway, but it's better to have it right here to not cause additional
confusion.
2024-04-16 18:19:47 +02:00
Jonas Schäfer 054447d147 jid: add more testcases
Because why not!
2024-04-15 19:54:59 +02:00
Jonas Schäfer c08946aa7c jid: skip `at` and `slash` in comparison operators
They only contain cached information and thus don't need to be included
in comparison and identity operators.

Fixes #123.
2024-04-15 19:35:02 +02:00
Jonas Schäfer c895cb1009 jid: implement Borrow<Jid> on FullJid and BareJid
This allows to use them interchangably when looking up keys in hash sets
and dicts.
2024-04-15 18:21:24 +02:00
Jonas Schäfer 2e9c9411a3 jid: rewrite public types
This moves InnerJid into Jid and reformulates BareJid and FullJid in
terms of Jid.

Doing this has the key advantage that FullJid and BareJid can deref to
and borrow as Jid. This, in turn, has the advantage that they can be
used much more flexibly in HashMaps. However, this is (as we say in
Germany) future music; this commit only does the internal reworking.

Oh and also, it saves 20% memory on Jid objects.

Fixes #122 more thoroughly, or rather the original intent behind it.
2024-04-15 18:21:24 +02:00
Jonas Schäfer 6344649deb Ignore missing disco#info feature in disco#info responses
xmpp-rs normally has the stance to get buggy implementations fixed
rather than dropping checks. In this particular case I think this is not
a good use of resources:

- The disco#info feature var conveys no actual information:
  If an implementation replies properly to a disco#info query, it is
  already implied that it supports the protocol.

- There are broken server implementations out there.
  A lot of them (all recent (>= 0.10 && < 0.13 AFAICT) Prosody IM
  instances). At this point in time, xmpp-rs is unable to query
  disco#info from MUCs hosted on such prosody versions, except by
  workarounds (such as the one removed in this diff).

The case would be different if there were no (deployed) implementations
which had this bug or if the bug actually had an effect on clients.
Especially the latter is not the case though, as pointed out above.

Hence, I conclude that this check is overly pedantic, and while it is
true to the letter of XEP-0030, the resources (time, emotional energy of
dealing with bugs, punching patches through to stable distributions,
etc. etc.) spent on getting this fixed would be better invested
elsewhere.

In addition, the workaround is extremely ugly and, even in the xmpp-rs
implementation, has no test coverage. Without test coverage of such an
implementation, it is bound to break in funny ways when xmpp-rs changes
the strings of its error messages (which is something one might do even
outside a breaking release).
2024-03-10 17:32:13 +01:00
23 changed files with 677 additions and 509 deletions

View File

@ -1,180 +0,0 @@
// Copyright (c) 2023 Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
#![deny(missing_docs)]
//! Provides a type for Jabber IDs.
//!
//! For usage, check the documentation on the `Jid` struct.
use crate::Error;
use core::num::NonZeroU16;
use memchr::memchr;
use std::borrow::Cow;
use std::str::FromStr;
use stringprep::{nameprep, nodeprep, resourceprep};
use crate::parts::{DomainRef, NodeRef, ResourceRef};
fn length_check(len: usize, error_empty: Error, error_too_long: Error) -> Result<(), Error> {
if len == 0 {
Err(error_empty)
} else if len > 1023 {
Err(error_too_long)
} else {
Ok(())
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub(crate) struct InnerJid {
pub(crate) normalized: String,
pub(crate) at: Option<NonZeroU16>,
pub(crate) slash: Option<NonZeroU16>,
}
impl InnerJid {
pub(crate) fn new(unnormalized: &str) -> Result<InnerJid, Error> {
let bytes = unnormalized.as_bytes();
let mut orig_at = memchr(b'@', bytes);
let mut orig_slash = memchr(b'/', bytes);
if orig_at.is_some() && orig_slash.is_some() && orig_at > orig_slash {
// This is part of the resource, not a node@domain separator.
orig_at = None;
}
let normalized = match (orig_at, orig_slash) {
(Some(at), Some(slash)) => {
let node = nodeprep(&unnormalized[..at]).map_err(|_| Error::NodePrep)?;
length_check(node.len(), Error::NodeEmpty, Error::NodeTooLong)?;
let domain = nameprep(&unnormalized[at + 1..slash]).map_err(|_| Error::NamePrep)?;
length_check(domain.len(), Error::DomainEmpty, Error::DomainTooLong)?;
let resource =
resourceprep(&unnormalized[slash + 1..]).map_err(|_| Error::ResourcePrep)?;
length_check(resource.len(), Error::ResourceEmpty, Error::ResourceTooLong)?;
orig_at = Some(node.len());
orig_slash = Some(node.len() + domain.len() + 1);
match (node, domain, resource) {
(Cow::Borrowed(_), Cow::Borrowed(_), Cow::Borrowed(_)) => {
unnormalized.to_string()
}
(node, domain, resource) => format!("{node}@{domain}/{resource}"),
}
}
(Some(at), None) => {
let node = nodeprep(&unnormalized[..at]).map_err(|_| Error::NodePrep)?;
length_check(node.len(), Error::NodeEmpty, Error::NodeTooLong)?;
let domain = nameprep(&unnormalized[at + 1..]).map_err(|_| Error::NamePrep)?;
length_check(domain.len(), Error::DomainEmpty, Error::DomainTooLong)?;
orig_at = Some(node.len());
match (node, domain) {
(Cow::Borrowed(_), Cow::Borrowed(_)) => unnormalized.to_string(),
(node, domain) => format!("{node}@{domain}"),
}
}
(None, Some(slash)) => {
let domain = nameprep(&unnormalized[..slash]).map_err(|_| Error::NamePrep)?;
length_check(domain.len(), Error::DomainEmpty, Error::DomainTooLong)?;
let resource =
resourceprep(&unnormalized[slash + 1..]).map_err(|_| Error::ResourcePrep)?;
length_check(resource.len(), Error::ResourceEmpty, Error::ResourceTooLong)?;
orig_slash = Some(domain.len());
match (domain, resource) {
(Cow::Borrowed(_), Cow::Borrowed(_)) => unnormalized.to_string(),
(domain, resource) => format!("{domain}/{resource}"),
}
}
(None, None) => {
let domain = nameprep(unnormalized).map_err(|_| Error::NamePrep)?;
length_check(domain.len(), Error::DomainEmpty, Error::DomainTooLong)?;
domain.into_owned()
}
};
Ok(InnerJid {
normalized,
at: orig_at.and_then(|x| NonZeroU16::new(x as u16)),
slash: orig_slash.and_then(|x| NonZeroU16::new(x as u16)),
})
}
pub(crate) fn node(&self) -> Option<&NodeRef> {
self.at.map(|at| {
let at = u16::from(at) as usize;
NodeRef::from_str_unchecked(&self.normalized[..at])
})
}
pub(crate) fn domain(&self) -> &DomainRef {
match (self.at, self.slash) {
(Some(at), Some(slash)) => {
let at = u16::from(at) as usize;
let slash = u16::from(slash) as usize;
DomainRef::from_str_unchecked(&self.normalized[at + 1..slash])
}
(Some(at), None) => {
let at = u16::from(at) as usize;
DomainRef::from_str_unchecked(&self.normalized[at + 1..])
}
(None, Some(slash)) => {
let slash = u16::from(slash) as usize;
DomainRef::from_str_unchecked(&self.normalized[..slash])
}
(None, None) => DomainRef::from_str_unchecked(&self.normalized),
}
}
pub(crate) fn resource(&self) -> Option<&ResourceRef> {
self.slash.map(|slash| {
let slash = u16::from(slash) as usize;
ResourceRef::from_str_unchecked(&self.normalized[slash + 1..])
})
}
#[inline(always)]
pub(crate) fn as_str(&self) -> &str {
self.normalized.as_str()
}
}
impl FromStr for InnerJid {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
InnerJid::new(s)
}
}
#[cfg(test)]
mod tests {
use super::*;
macro_rules! assert_size (
($t:ty, $sz:expr) => (
assert_eq!(::std::mem::size_of::<$t>(), $sz);
);
);
#[cfg(target_pointer_width = "32")]
#[test]
fn test_size() {
assert_size!(InnerJid, 16);
}
#[cfg(target_pointer_width = "64")]
#[test]
fn test_size() {
assert_size!(InnerJid, 32);
}
}

File diff suppressed because it is too large Load Diff

View File

@ -5,7 +5,7 @@ use std::str::FromStr;
use stringprep::{nameprep, nodeprep, resourceprep};
use crate::{BareJid, Error, InnerJid, Jid};
use crate::{BareJid, Error, Jid};
fn length_check(len: usize, error_empty: Error, error_too_long: Error) -> Result<(), Error> {
if len == 0 {
@ -250,18 +250,18 @@ impl DomainRef {
impl From<DomainPart> for BareJid {
fn from(other: DomainPart) -> Self {
BareJid {
inner: InnerJid {
normalized: other.0,
at: None,
slash: None,
},
inner: other.into(),
}
}
}
impl From<DomainPart> for Jid {
fn from(other: DomainPart) -> Self {
Jid::Bare(other.into())
Jid {
normalized: other.0,
at: None,
slash: None,
}
}
}

View File

@ -88,7 +88,7 @@ impl From<BindResponse> for FullJid {
impl From<BindResponse> for Jid {
fn from(bind: BindResponse) -> Jid {
Jid::Full(bind.jid)
Jid::from(bind.jid)
}
}

View File

@ -71,8 +71,8 @@ mod tests {
assert_size!(Enable, 0);
assert_size!(Disable, 0);
assert_size!(Private, 0);
assert_size!(Received, 152);
assert_size!(Sent, 152);
assert_size!(Received, 140);
assert_size!(Sent, 140);
}
#[cfg(target_pointer_width = "64")]
@ -81,8 +81,8 @@ mod tests {
assert_size!(Enable, 0);
assert_size!(Disable, 0);
assert_size!(Private, 0);
assert_size!(Received, 288);
assert_size!(Sent, 288);
assert_size!(Received, 264);
assert_size!(Sent, 264);
}
#[test]

View File

@ -40,13 +40,13 @@ mod tests {
#[cfg(target_pointer_width = "32")]
#[test]
fn test_size() {
assert_size!(Delay, 48);
assert_size!(Delay, 44);
}
#[cfg(target_pointer_width = "64")]
#[test]
fn test_size() {
assert_size!(Delay, 80);
assert_size!(Delay, 72);
}
#[test]

View File

@ -161,13 +161,6 @@ impl TryFrom<Element> for DiscoInfoResult {
"There must be at least one feature in disco#info.",
));
}
if !result.features.contains(&Feature {
var: ns::DISCO_INFO.to_owned(),
}) {
return Err(Error::ParseError(
"disco#info feature not present in disco#info.",
));
}
Ok(result)
}
@ -248,7 +241,7 @@ mod tests {
assert_size!(DiscoInfoQuery, 12);
assert_size!(DiscoInfoResult, 48);
assert_size!(Item, 44);
assert_size!(Item, 40);
assert_size!(DiscoItemsQuery, 52);
assert_size!(DiscoItemsResult, 64);
}
@ -261,7 +254,7 @@ mod tests {
assert_size!(DiscoInfoQuery, 24);
assert_size!(DiscoInfoResult, 96);
assert_size!(Item, 88);
assert_size!(Item, 80);
assert_size!(DiscoItemsQuery, 104);
assert_size!(DiscoItemsResult, 128);
}
@ -400,14 +393,6 @@ mod tests {
_ => panic!(),
};
assert_eq!(message, "There must be at least one feature in disco#info.");
let elem: Element = "<query xmlns='http://jabber.org/protocol/disco#info'><identity category='client' type='pc'/><feature var='http://jabber.org/protocol/disco#items'/></query>".parse().unwrap();
let error = DiscoInfoResult::try_from(elem).unwrap_err();
let message = match error {
Error::ParseError(string) => string,
_ => panic!(),
};
assert_eq!(message, "disco#info feature not present in disco#info.");
}
#[test]

View File

@ -32,13 +32,13 @@ mod tests {
#[cfg(target_pointer_width = "32")]
#[test]
fn test_size() {
assert_size!(Forwarded, 152);
assert_size!(Forwarded, 140);
}
#[cfg(target_pointer_width = "64")]
#[test]
fn test_size() {
assert_size!(Forwarded, 288);
assert_size!(Forwarded, 264);
}
#[test]

View File

@ -232,15 +232,15 @@ mod tests {
#[cfg(target_pointer_width = "32")]
#[test]
fn test_size() {
assert_size!(IqType, 96);
assert_size!(Iq, 148);
assert_size!(IqType, 92);
assert_size!(Iq, 156);
}
#[cfg(target_pointer_width = "64")]
#[test]
fn test_size() {
assert_size!(IqType, 192);
assert_size!(Iq, 296);
assert_size!(IqType, 184);
assert_size!(Iq, 272);
}
#[test]

View File

@ -46,14 +46,14 @@ mod tests {
#[test]
fn test_size() {
assert_size!(JidPrepQuery, 12);
assert_size!(JidPrepResponse, 20);
assert_size!(JidPrepResponse, 16);
}
#[cfg(target_pointer_width = "64")]
#[test]
fn test_size() {
assert_size!(JidPrepQuery, 24);
assert_size!(JidPrepResponse, 40);
assert_size!(JidPrepResponse, 32);
}
#[test]

View File

@ -692,7 +692,7 @@ mod tests {
assert_size!(Reason, 1);
assert_size!(ReasonElement, 16);
assert_size!(SessionId, 12);
assert_size!(Jingle, 112);
assert_size!(Jingle, 104);
}
#[cfg(target_pointer_width = "64")]
@ -711,7 +711,7 @@ mod tests {
assert_size!(Reason, 1);
assert_size!(ReasonElement, 32);
assert_size!(SessionId, 24);
assert_size!(Jingle, 224);
assert_size!(Jingle, 208);
}
#[test]

View File

@ -284,7 +284,7 @@ mod tests {
assert_size!(Mode, 1);
assert_size!(CandidateId, 12);
assert_size!(StreamId, 12);
assert_size!(Candidate, 60);
assert_size!(Candidate, 56);
assert_size!(TransportPayload, 16);
assert_size!(Transport, 44);
}
@ -296,7 +296,7 @@ mod tests {
assert_size!(Mode, 1);
assert_size!(CandidateId, 24);
assert_size!(StreamId, 24);
assert_size!(Candidate, 96);
assert_size!(Candidate, 88);
assert_size!(TransportPayload, 32);
assert_size!(Transport, 88);
}

View File

@ -166,7 +166,7 @@ mod tests {
fn test_size() {
assert_size!(QueryId, 12);
assert_size!(Query, 120);
assert_size!(Result_, 176);
assert_size!(Result_, 164);
assert_size!(Complete, 1);
assert_size!(Fin, 44);
}
@ -176,7 +176,7 @@ mod tests {
fn test_size() {
assert_size!(QueryId, 24);
assert_size!(Query, 240);
assert_size!(Result_, 336);
assert_size!(Result_, 312);
assert_size!(Complete, 1);
assert_size!(Fin, 88);
}

View File

@ -307,7 +307,7 @@ mod tests {
assert_size!(Body, 12);
assert_size!(Subject, 12);
assert_size!(Thread, 12);
assert_size!(Message, 104);
assert_size!(Message, 96);
}
#[cfg(target_pointer_width = "64")]
@ -317,7 +317,7 @@ mod tests {
assert_size!(Body, 24);
assert_size!(Subject, 24);
assert_size!(Thread, 24);
assert_size!(Message, 208);
assert_size!(Message, 192);
}
#[test]

View File

@ -379,7 +379,7 @@ mod tests {
fn test_size() {
assert_size!(Show, 1);
assert_size!(Type, 1);
assert_size!(Presence, 80);
assert_size!(Presence, 72);
}
#[cfg(target_pointer_width = "64")]
@ -387,7 +387,7 @@ mod tests {
fn test_size() {
assert_size!(Show, 1);
assert_size!(Type, 1);
assert_size!(Presence, 160);
assert_size!(Presence, 144);
}
#[test]

View File

@ -200,11 +200,11 @@ mod tests {
node: NodeName(String::from("foo")),
affiliations: vec![
Affiliation {
jid: Jid::Bare(BareJid::from_str("hamlet@denmark.lit").unwrap()),
jid: Jid::from(BareJid::from_str("hamlet@denmark.lit").unwrap()),
affiliation: AffiliationAttribute::Owner,
},
Affiliation {
jid: Jid::Bare(BareJid::from_str("polonius@denmark.lit").unwrap()),
jid: Jid::from(BareJid::from_str("polonius@denmark.lit").unwrap()),
affiliation: AffiliationAttribute::Outcast,
},
],
@ -335,22 +335,22 @@ mod tests {
node: NodeName(String::from("foo")),
subscriptions: vec![
SubscriptionElem {
jid: Jid::Bare(BareJid::from_str("hamlet@denmark.lit").unwrap()),
jid: Jid::from(BareJid::from_str("hamlet@denmark.lit").unwrap()),
subscription: Subscription::Subscribed,
subid: None,
},
SubscriptionElem {
jid: Jid::Bare(BareJid::from_str("polonius@denmark.lit").unwrap()),
jid: Jid::from(BareJid::from_str("polonius@denmark.lit").unwrap()),
subscription: Subscription::Unconfigured,
subid: None,
},
SubscriptionElem {
jid: Jid::Bare(BareJid::from_str("bernardo@denmark.lit").unwrap()),
jid: Jid::from(BareJid::from_str("bernardo@denmark.lit").unwrap()),
subscription: Subscription::Subscribed,
subid: Some(String::from("123-abc")),
},
SubscriptionElem {
jid: Jid::Bare(BareJid::from_str("bernardo@denmark.lit").unwrap()),
jid: Jid::from(BareJid::from_str("bernardo@denmark.lit").unwrap()),
subscription: Subscription::Subscribed,
subid: Some(String::from("004-yyy")),
},

View File

@ -319,7 +319,7 @@ mod tests {
fn test_size() {
assert_size!(ErrorType, 1);
assert_size!(DefinedCondition, 1);
assert_size!(StanzaError, 96);
assert_size!(StanzaError, 92);
}
#[cfg(target_pointer_width = "64")]
@ -327,7 +327,7 @@ mod tests {
fn test_size() {
assert_size!(ErrorType, 1);
assert_size!(DefinedCondition, 1);
assert_size!(StanzaError, 192);
assert_size!(StanzaError, 184);
}
#[test]

View File

@ -44,14 +44,14 @@ mod tests {
#[cfg(target_pointer_width = "32")]
#[test]
fn test_size() {
assert_size!(StanzaId, 32);
assert_size!(StanzaId, 24);
assert_size!(OriginId, 12);
}
#[cfg(target_pointer_width = "64")]
#[test]
fn test_size() {
assert_size!(StanzaId, 64);
assert_size!(StanzaId, 56);
assert_size!(OriginId, 24);
}

View File

@ -75,7 +75,7 @@ async fn main() -> Result<(), Option<()>> {
Event::RoomJoined(jid) => {
println!("Joined room {}.", jid);
client
.send_message(Jid::Bare(jid), MessageType::Groupchat, "en", "Hello world!")
.send_message(Jid::from(jid), MessageType::Groupchat, "en", "Hello world!")
.await;
}
Event::RoomLeft(jid) => {

View File

@ -8,58 +8,17 @@ use tokio_xmpp::connect::ServerConnector;
use tokio_xmpp::{
parsers::{
bookmarks,
disco::{DiscoInfoResult, Feature},
disco::DiscoInfoResult,
iq::Iq,
ns,
private::Query as PrivateXMLQuery,
pubsub::pubsub::{Items, PubSub},
Error as ParsersError,
},
Element, Jid,
Jid,
};
use crate::Agent;
// This method is a workaround due to prosody bug https://issues.prosody.im/1664
// FIXME: To be removed in the future
// The server doesn't return disco#info feature when querying the account
// so we add it manually because we know it's true
pub async fn handle_disco_info_result_payload<C: ServerConnector>(
agent: &mut Agent<C>,
payload: Element,
from: Jid,
) {
match DiscoInfoResult::try_from(payload.clone()) {
Ok(disco) => {
handle_disco_info_result(agent, disco, from).await;
}
Err(e) => match e {
ParsersError::ParseError(reason) => {
if reason == "disco#info feature not present in disco#info." {
let mut payload = payload.clone();
let disco_feature =
Feature::new("http://jabber.org/protocol/disco#info").into();
payload.append_child(disco_feature);
match DiscoInfoResult::try_from(payload) {
Ok(disco) => {
handle_disco_info_result(agent, disco, from).await;
}
Err(e) => {
panic!("Wrong disco#info format after workaround: {}", e)
}
}
} else {
panic!(
"Wrong disco#info format (reason cannot be worked around): {}",
e
)
}
}
_ => panic!("Wrong disco#info format: {}", e),
},
}
}
pub async fn handle_disco_info_result<C: ServerConnector>(
agent: &mut Agent<C>,
disco: DiscoInfoResult,

View File

@ -6,7 +6,7 @@
use tokio_xmpp::connect::ServerConnector;
use tokio_xmpp::{
parsers::{ns, private::Query as PrivateXMLQuery, roster::Roster},
parsers::{disco::DiscoInfoResult, ns, private::Query as PrivateXMLQuery, roster::Roster},
Element, Jid,
};
@ -46,6 +46,13 @@ pub async fn handle_iq_result<C: ServerConnector>(
}
}
} else if payload.is("query", ns::DISCO_INFO) {
disco::handle_disco_info_result_payload(agent, payload, from).await;
match DiscoInfoResult::try_from(payload.clone()) {
Ok(disco) => {
disco::handle_disco_info_result(agent, disco, from).await;
}
Err(e) => match e {
_ => panic!("Wrong disco#info format: {}", e),
},
}
}
}

View File

@ -25,8 +25,8 @@ pub async fn handle_message_chat<C: ServerConnector>(
for payload in &message.payloads {
if let Ok(_) = MucUser::try_from(payload.clone()) {
let event = match from.clone() {
Jid::Bare(bare) => {
let event = match from.clone().try_into_full() {
Err(bare) => {
// TODO: Can a service message be of type Chat/Normal and not Groupchat?
warn!("Received misformed MessageType::Chat in muc#user namespace from a bare JID.");
Event::ServiceMessage(
@ -36,7 +36,7 @@ pub async fn handle_message_chat<C: ServerConnector>(
time_info.clone(),
)
}
Jid::Full(full) => Event::RoomPrivateMessage(
Ok(full) => Event::RoomPrivateMessage(
message.id.clone(),
full.to_bare(),
full.resource().to_string(),

View File

@ -28,17 +28,15 @@ pub async fn handle_message_group_chat<C: ServerConnector>(
}
if let Some((_lang, body)) = message.get_best_body(langs) {
let event = match from.clone() {
Jid::Full(full) => Event::RoomMessage(
let event = match from.clone().try_into_full() {
Ok(full) => Event::RoomMessage(
message.id.clone(),
from.to_bare(),
full.resource().to_string(),
body.clone(),
time_info,
),
Jid::Bare(bare) => {
Event::ServiceMessage(message.id.clone(), bare, body.clone(), time_info)
}
Err(bare) => Event::ServiceMessage(message.id.clone(), bare, body.clone(), time_info),
};
events.push(event)
}