mirror of
https://gitlab.com/xmpp-rs/xmpp-rs.git
synced 2024-07-12 22:21:53 +00:00
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). - XEP-0030 now features a note which reads: > Note: Some entities are known not to advertise the > `http://jabber.org/protocol/disco#info` feature within their > responses, contrary to this specification. Entities receiving > otherwise valid responses which do not include this feature SHOULD > infer the support. 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 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).
This commit is contained in:
parent
eb0bc1b82f
commit
80efd2eb19
3 changed files with 11 additions and 60 deletions
|
@ -163,13 +163,6 @@ impl TryFrom<Element> for DiscoInfoResult {
|
||||||
"There must be at least one feature in disco#info.",
|
"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)
|
Ok(result)
|
||||||
|
@ -403,14 +396,6 @@ mod tests {
|
||||||
_ => panic!(),
|
_ => panic!(),
|
||||||
};
|
};
|
||||||
assert_eq!(message, "There must be at least one feature in disco#info.");
|
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]
|
#[test]
|
||||||
|
|
|
@ -8,58 +8,17 @@ use tokio_xmpp::connect::ServerConnector;
|
||||||
use tokio_xmpp::{
|
use tokio_xmpp::{
|
||||||
parsers::{
|
parsers::{
|
||||||
bookmarks,
|
bookmarks,
|
||||||
disco::{DiscoInfoResult, Feature},
|
disco::DiscoInfoResult,
|
||||||
iq::Iq,
|
iq::Iq,
|
||||||
ns,
|
ns,
|
||||||
private::Query as PrivateXMLQuery,
|
private::Query as PrivateXMLQuery,
|
||||||
pubsub::pubsub::{Items, PubSub},
|
pubsub::pubsub::{Items, PubSub},
|
||||||
Error as ParsersError,
|
|
||||||
},
|
},
|
||||||
Element, Jid,
|
Jid,
|
||||||
};
|
};
|
||||||
|
|
||||||
use crate::Agent;
|
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>(
|
pub async fn handle_disco_info_result<C: ServerConnector>(
|
||||||
agent: &mut Agent<C>,
|
agent: &mut Agent<C>,
|
||||||
disco: DiscoInfoResult,
|
disco: DiscoInfoResult,
|
||||||
|
|
|
@ -6,7 +6,7 @@
|
||||||
|
|
||||||
use tokio_xmpp::connect::ServerConnector;
|
use tokio_xmpp::connect::ServerConnector;
|
||||||
use tokio_xmpp::{
|
use tokio_xmpp::{
|
||||||
parsers::{ns, private::Query as PrivateXMLQuery, roster::Roster},
|
parsers::{disco::DiscoInfoResult, ns, private::Query as PrivateXMLQuery, roster::Roster},
|
||||||
Element, Jid,
|
Element, Jid,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -46,6 +46,13 @@ pub async fn handle_iq_result<C: ServerConnector>(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if payload.is("query", ns::DISCO_INFO) {
|
} 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),
|
||||||
|
},
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue