diff --git a/CHANGELOG.md b/CHANGELOG.md index 80c7fd04e..2029b9819 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/location.rs b/src/location.rs index 10e6a77fd..c3c8e59ac 100644 --- a/src/location.rs +++ b/src/location.rs @@ -63,7 +63,7 @@ impl Kml { Default::default() } - pub fn parse(context: &Context, to_parse: &[u8]) -> Result { + pub fn parse(to_parse: &[u8]) -> Result { 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) { - Ok(quick_xml::events::Event::Start(ref e)) => kml.starttag_cb(e, &reader), - Ok(quick_xml::events::Event::End(ref e)) => kml.endtag_cb(e), - Ok(quick_xml::events::Event::Text(ref e)) => kml.text_cb(e, &reader), - Err(e) => { - error!( - context, - "Location parsing: Error at position {}: {:?}", - reader.buffer_position(), - e - ); - } - Ok(quick_xml::events::Event::Eof) => break, + match reader.read_event(&mut buf).with_context(|| { + format!( + "location parsing error at position {}", + reader.buffer_position() + ) + })? { + quick_xml::events::Event::Start(ref e) => kml.starttag_cb(e, &reader), + quick_xml::events::Event::End(ref e) => kml.endtag_cb(e), + quick_xml::events::Event::Text(ref e) => kml.text_cb(e, &reader), + 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)] - async fn test_kml_parse() { - let context = TestContext::new().await; - + #[test] + fn test_kml_parse() { let xml = b"\n\n\n2019-03-06T21:09:57Z9.423110,53.790302\n\n \n\t2018-12-13T22:11:12Z\t 19.423110 \t , \n 63.790302\n \n\n"; - 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)] - async fn test_get_message_kml() { - let context = TestContext::new().await; + #[test] + fn test_kml_parse_error() { + let xml = b"?"; + 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); diff --git a/src/mimeparser.rs b/src/mimeparser.rs index e61c64891..0ed618956 100644 --- a/src/mimeparser.rs +++ b/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); })