location: don't ignore KML parsing errors

CHANGELOG.md

@@ -12,6 +12,7 @@
 ### Fixes
 - mailing list: remove square-brackets only for first name #3452
 - do not use footers from mailinglists as the contact status #3460
+- don't ignore KML parsing errors #3473
 
 
 ## 1.87.0

src/location.rs

@@ -63,7 +63,7 @@ impl Kml {
         Default::default()
     }
 
-    pub fn parse(context: &Context, to_parse: &[u8]) -> Result<Self> {
+    pub fn parse(to_parse: &[u8]) -> Result<Self> {
         ensure!(to_parse.len() <= 1024 * 1024, "kml-file is too large");
 
         let mut reader = quick_xml::Reader::from_reader(to_parse);
@@ -75,19 +75,16 @@ impl Kml {
         let mut buf = Vec::new();
 
         loop {
-            match reader.read_event(&mut buf) {
+            match reader.read_event(&mut buf).with_context(|| {
-                Ok(quick_xml::events::Event::Start(ref e)) => kml.starttag_cb(e, &reader),
+                format!(
-                Ok(quick_xml::events::Event::End(ref e)) => kml.endtag_cb(e),
+                    "location parsing error at position {}",
-                Ok(quick_xml::events::Event::Text(ref e)) => kml.text_cb(e, &reader),
+                    reader.buffer_position()
-                Err(e) => {
+                )
-                    error!(
+            })? {
-                        context,
+                quick_xml::events::Event::Start(ref e) => kml.starttag_cb(e, &reader),
-                        "Location parsing: Error at position {}: {:?}",
+                quick_xml::events::Event::End(ref e) => kml.endtag_cb(e),
-                        reader.buffer_position(),
+                quick_xml::events::Event::Text(ref e) => kml.text_cb(e, &reader),
-                        e
+                quick_xml::events::Event::Eof => break,
-                    );
-                }
-                Ok(quick_xml::events::Event::Eof) => break,
                 _ => (),
             }
             buf.clear();
@@ -731,14 +728,12 @@ mod tests {
     use crate::receive_imf::receive_imf;
     use crate::test_utils::TestContext;
 
-    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    #[test]
-    async fn test_kml_parse() {
+    fn test_kml_parse() {
-        let context = TestContext::new().await;
-
         let xml =
             b"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<kml xmlns=\"http://www.opengis.net/kml/2.2\">\n<Document addr=\"user@example.org\">\n<Placemark><Timestamp><when>2019-03-06T21:09:57Z</when></Timestamp><Point><coordinates accuracy=\"32.000000\">9.423110,53.790302</coordinates></Point></Placemark>\n<PlaceMARK>\n<Timestamp><WHEN > \n\t2018-12-13T22:11:12Z\t</WHEN></Timestamp><Point><coordinates aCCuracy=\"2.500000\"> 19.423110 \t , \n 63.790302\n </coordinates></Point></PlaceMARK>\n</Document>\n</kml>";
 
-        let kml = Kml::parse(&context.ctx, xml).expect("parsing failed");
+        let kml = Kml::parse(xml).expect("parsing failed");
 
         assert!(kml.addr.is_some());
         assert_eq!(kml.addr.as_ref().unwrap(), "user@example.org",);
@@ -763,13 +758,18 @@ mod tests {
         assert_eq!(locations_ref[1].timestamp, 1544739072);
     }
 
-    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    #[test]
-    async fn test_get_message_kml() {
+    fn test_kml_parse_error() {
-        let context = TestContext::new().await;
+        let xml = b"<?><xmlversi\"\"\">?</document>";
+        assert!(Kml::parse(xml).is_err());
+    }
+
+    #[test]
+    fn test_get_message_kml() {
         let timestamp = 1598490000;
 
         let xml = get_message_kml(timestamp, 51.423723f64, 8.552556f64);
-        let kml = Kml::parse(&context.ctx, xml.as_bytes()).expect("parsing failed");
+        let kml = Kml::parse(xml.as_bytes()).expect("parsing failed");
         let locations_ref = &kml.locations;
         assert_eq!(locations_ref.len(), 1);
 

src/mimeparser.rs

@@ -1038,7 +1038,7 @@ impl MimeMessage {
             // XXX what if somebody sends eg an "location-highlights.kml"
             // attachment unrelated to location streaming?
             if filename.starts_with("location") || filename.starts_with("message") {
-                let parsed = location::Kml::parse(context, decoded_data)
+                let parsed = location::Kml::parse(decoded_data)
                     .map_err(|err| {
                         warn!(context, "failed to parse kml part: {}", err);
                     })