🦺 Drop inefficient validation of DOCTYPE/root

dbus2#159 (comment)

src/config.rs

@@ -1,7 +1,6 @@
 use std::{collections::HashSet, path::PathBuf, time::Duration};
 
 use anyhow::{Error, Result};
-use quick_xml::{events::Event, Reader};
 use serde::Deserialize;
 
 mod xml;
@@ -11,24 +10,13 @@ use xml::{
     Document, Element, PolicyContext, PolicyElement, RuleAttributes, RuleElement, TypeElement,
 };
 
-const EXPECTED_DOCTYPE_PARTS: &[&str] = &[
-    "busconfig",
-    "PUBLIC",
-    r#""-//freedesktop//DTD"#,
-    "D-Bus",
-    "Bus",
-    "Configuration",
-    r#"1.0//EN""#,
-    r#""http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd""#,
-];
-
 const STANDARD_SYSTEM_SERVICEDIRS: &[&str] = &[
     "/usr/local/share/dbus-1/system-services",
     "/usr/share/dbus-1/system-services",
     "/lib/dbus-1/system-services",
 ];
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
 pub enum Access {
     Allow,
     Deny,
@@ -41,7 +29,6 @@ pub enum Access {
 ///
 /// [XML configuration files]: https://dbus.freedesktop.org/doc/dbus-daemon.1.html#configuration_file
 #[derive(Clone, Debug, Default, Deserialize, PartialEq)]
-#[serde(try_from = "Document")]
 pub struct BusConfig {
     /// If `true`, connections that authenticated using the ANONYMOUS mechanism will be authorized
     /// to connect. This option has no practical effect unless the ANONYMOUS mechanism has also
@@ -213,43 +200,9 @@ impl BusConfig {
     }
 
     pub fn parse(s: &str) -> Result<Self> {
-        // validate that our DOCTYPE and root element are correct
-        let mut reader = Reader::from_reader(s.as_bytes());
-        let mut has_dtd = false;
-        loop {
-            match reader.read_event()? {
-                Event::DocType(bytes_text) => {
-                    has_dtd = true;
-                    let content = bytes_text.unescape()?;
-                    // this approach does throw away extra whitespace within quoted strings,
-                    // which means we do allow through some unexpected DOCTYPEs
-                    // TODO: consider whether added complexity is worth 100% strictness for DOCTYPEs
-                    let dtd_parts: Vec<&str> = content.as_ref().split_whitespace().collect();
-                    if dtd_parts != EXPECTED_DOCTYPE_PARTS {
-                        assert_eq!(dtd_parts, EXPECTED_DOCTYPE_PARTS);
-                        return Err(Error::msg(
-                            "incorrect/incomplete `<!DOCTYPE ...> declaration`",
-                        ));
-                    }
-                    // we currently don't bother with the case of multiple DTDs
-                }
-                Event::Start(bytes_start) => {
-                    if !has_dtd {
-                        return Err(Error::msg(
-                            "must specify `<!DOCTYPE ...>` before root element",
-                        ));
-                    }
-                    if bytes_start.name().as_ref() != b"busconfig" {
-                        return Err(Error::msg("root element must be `<busconfig>`"));
-                    }
-                    break;
-                }
-                Event::Eof => break,
-                _ => {}
-            }
-        }
-
-        quick_xml::de::from_str(s).map_err(Error::from)
+        // TODO: efficiently validate that our DOCTYPE and root element are correct
+        let doc: Document = quick_xml::de::from_str(s)?;
+        Self::try_from(doc)
     }
 }
 
@@ -310,7 +263,7 @@ pub enum MessageType {
     Error,
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
 pub enum Operation {
     /// rules checked when a new connection to the message bus is established
     Connect,
@@ -376,7 +329,7 @@ impl TryFrom<RuleAttributes> for OptionalOperation {
     }
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
 pub enum Policy {
     DefaultContext(Vec<Rule>),
     Group(Vec<Rule>, String),
@@ -442,7 +395,7 @@ impl TryFrom<PolicyElement> for OptionalPolicy {
     }
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
 pub struct ReceiveOperation {
     pub error: String,
     pub interface: String,
@@ -584,7 +537,7 @@ fn rules_try_from_rule_elements(value: Vec<RuleElement>) -> Result<Vec<Rule>> {
 
 pub type Rule = (Access, Operation);
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
 pub struct SendOperation {
     pub broadcast: Option<bool>,
     pub destination: String,
@@ -631,34 +584,6 @@ impl From<RuleAttributes> for SendOperation {
 mod tests {
     use super::*;
 
-    #[test]
-    #[should_panic]
-    fn bus_config_from_str_without_dtd_error() {
-        let input = r#"<busconfig></busconfig>"#;
-        BusConfig::parse(input).expect("should parse XML input");
-    }
-
-    #[test]
-    #[should_panic]
-    fn bus_config_parse_with_unexpected_dtd_error() {
-        let input = r#"<!DOCTYPE busconfig PUBLIC
-        "-//W3C//DTD XHTML Basic 1.1//EN"
-        "http://www.w3.org/TR/xhtml-basic/xhtml-basic11.dtd">
-        <busconfig></busconfig>
-        "#;
-        BusConfig::parse(input).expect("should parse XML input");
-    }
-
-    #[test]
-    #[should_panic]
-    fn bus_config_parse_with_unexpected_root_element_error() {
-        let input = r#"<!DOCTYPE foo PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
-        "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
-        <foo></foo>
-        "#;
-        BusConfig::parse(input).expect("should parse XML input");
-    }
-
     #[test]
     fn bus_config_parse_with_dtd_and_root_element_ok() {
         let input = r#"<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"