let sending invalid webxdc fail (#2993)

* let sending invalid webxdc fail

invalid webxdc can still be send as Viewtype::File, however
(maybe one want to discuss errors or so ;)

* clarify the supported zip compression methods

CHANGELOG.md

@@ -35,7 +35,7 @@
 - sql: enable `auto_vacuum=INCREMENTAL` #2931
 - Synchronize Seen status across devices #2942
 - Add API to set the database password #2956
-- Add Webxdc #2826 #2998
+- Add Webxdc #2826 #2971 #2975 #2977 #2979 #2993 #2998
 
 ### Changed
 - selfstatus now defaults to empty

draft/webxdc-dev-reference.md

@@ -3,6 +3,8 @@
 ## Webxdc File Format
 
 - a **Webxdc app** is a **ZIP-file** with the extension `.xdc`
+- the ZIP-file must use the default compression methods as of RFC 1950,
+  this is "Deflate" or "Store"
 - the ZIP-file must contain at least the file `index.html`
 - if the Webxdc app is started, `index.html` is opened in a restricted webview
   that allow accessing resources only from the ZIP-file

src/chat.rs

@@ -25,8 +25,8 @@ use crate::context::Context;
 use crate::dc_receive_imf::ReceivedMsg;
 use crate::dc_tools::{
     dc_create_id, dc_create_outgoing_rfc724_mid, dc_create_smeared_timestamp,
-    dc_create_smeared_timestamps, dc_get_abs_path, dc_get_filebytes, dc_gm2local_offset,
+    dc_create_smeared_timestamps, dc_get_abs_path, dc_gm2local_offset, improve_single_line_input,
-    improve_single_line_input, time, IsNoneOrEmpty,
+    time, IsNoneOrEmpty,
 };
 use crate::ephemeral::{delete_expired_messages, schedule_ephemeral_task, Timer as EphemeralTimer};
 use crate::events::EventType;
@@ -41,7 +41,7 @@ use crate::peerstate::{Peerstate, PeerstateVerifiedStatus};
 use crate::scheduler::InterruptInfo;
 use crate::smtp::send_msg_to_smtp;
 use crate::stock_str;
-use crate::webxdc::{WEBXDC_SENDING_LIMIT, WEBXDC_SUFFIX};
+use crate::webxdc::WEBXDC_SUFFIX;
 
 /// An chat item, such as a message or a marker.
 #[derive(Debug, Copy, Clone)]
@@ -1836,32 +1836,27 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> {
             if let Some((better_type, better_mime)) =
                 message::guess_msgtype_from_suffix(&blob.to_abs_path())
             {
-                msg.viewtype = better_type;
+                if better_type != Viewtype::Webxdc
-                if !msg.param.exists(Param::MimeType) {
+                    || context
-                    msg.param.set(Param::MimeType, better_mime);
+                        .ensure_sendable_webxdc_file(&blob.to_abs_path())
+                        .await
+                        .is_ok()
+                {
+                    msg.viewtype = better_type;
+                    if !msg.param.exists(Param::MimeType) {
+                        msg.param.set(Param::MimeType, better_mime);
+                    }
                 }
             }
-        } else if !msg.param.exists(Param::MimeType) {
+        } else if msg.viewtype == Viewtype::Webxdc {
-            if let Some((_, mime)) = message::guess_msgtype_from_suffix(&blob.to_abs_path()) {
+            context
-                msg.param.set(Param::MimeType, mime);
+                .ensure_sendable_webxdc_file(&blob.to_abs_path())
-            }
+                .await?;
         }
 
-        if msg.viewtype == Viewtype::Webxdc {
+        if !msg.param.exists(Param::MimeType) {
-            if blob.suffix() != Some(WEBXDC_SUFFIX) {
+            if let Some((_, mime)) = message::guess_msgtype_from_suffix(&blob.to_abs_path()) {
-                bail!(
+                msg.param.set(Param::MimeType, mime);
-                    "webxdc message {} does not have suffix {}",
-                    blob,
-                    WEBXDC_SUFFIX
-                );
-            } else if dc_get_filebytes(context, blob.to_abs_path()).await
-                > WEBXDC_SENDING_LIMIT as u64
-            {
-                bail!(
-                    "webxdc message {} exceeds acceptable size of {} bytes",
-                    blob.as_name(),
-                    WEBXDC_SENDING_LIMIT
-                );
             }
         }
 

src/webxdc.rs

@@ -8,6 +8,7 @@ use crate::mimeparser::SystemMessage;
 use crate::param::Param;
 use crate::{chat, EventType};
 use anyhow::{bail, ensure, format_err, Result};
+use async_std::path::PathBuf;
 use lettre_email::mime::{self};
 use lettre_email::PartBuilder;
 use serde::{Deserialize, Serialize};
@@ -30,7 +31,7 @@ const WEBXDC_DEFAULT_ICON: &str = "__webxdc__/default-icon.png";
 ///
 /// The limit is also an experiment to see how small we can go;
 /// it is planned to raise that limit as needed in subsequent versions.
-pub(crate) const WEBXDC_SENDING_LIMIT: usize = 102400;
+const WEBXDC_SENDING_LIMIT: usize = 102400;
 
 /// Be more tolerant for .xdc sizes on receiving -
 /// might be, the senders version uses already a larger limit
@@ -98,6 +99,7 @@ pub(crate) struct StatusUpdateItem {
 }
 
 impl Context {
+    /// check if a file is an acceptable webxdc for sending or receiving.
     pub(crate) async fn is_webxdc_file(&self, filename: &str, buf: &[u8]) -> Result<bool> {
         if filename.ends_with(WEBXDC_SUFFIX) {
             let reader = std::io::Cursor::new(buf);
@@ -106,19 +108,47 @@ impl Context {
                     if buf.len() <= WEBXDC_RECEIVING_LIMIT {
                         return Ok(true);
                     } else {
-                        error!(
+                        info!(
                             self,
-                            "{} exceeds acceptable size of {} bytes.",
+                            "{} exceeds receiving limit of {} bytes",
                             &filename,
-                            WEBXDC_SENDING_LIMIT
+                            WEBXDC_RECEIVING_LIMIT
                         );
                     }
+                } else {
+                    info!(self, "{} misses index.html", &filename);
                 }
+            } else {
+                info!(self, "{} cannot be opened as zip-file", &filename);
             }
         }
         Ok(false)
     }
 
+    /// ensure that a file is an acceptable webxdc for sending
+    /// (sending has more strict size limits).
+    pub(crate) async fn ensure_sendable_webxdc_file(&self, path: &PathBuf) -> Result<()> {
+        let mut file = std::fs::File::open(path)?;
+        let mut buf = Vec::new();
+        file.read_to_end(&mut buf)?;
+        if !self
+            .is_webxdc_file(path.to_str().unwrap_or_default(), &buf)
+            .await?
+        {
+            bail!(
+                "{} is not a valid webxdc file",
+                path.to_str().unwrap_or_default()
+            );
+        } else if buf.len() > WEBXDC_SENDING_LIMIT {
+            bail!(
+                "webxdc {} exceeds acceptable size of {} bytes",
+                path.to_str().unwrap_or_default(),
+                WEBXDC_SENDING_LIMIT
+            );
+        }
+        Ok(())
+    }
+
     /// Takes an update-json as `{payload: PAYLOAD}` (or legacy `PAYLOAD`)
     /// writes it to the database and handles events, info-messages and summary.
     async fn create_status_update_record(
@@ -527,6 +557,7 @@ mod tests {
         )
         .await?;
         let instance_msg_id = send_msg(t, chat_id, &mut instance).await?;
+        assert_eq!(instance.viewtype, Viewtype::Webxdc);
         Message::load_from_db(t, instance_msg_id).await
     }
 
@@ -554,6 +585,38 @@ mod tests {
         Ok(())
     }
 
+    #[async_std::test]
+    async fn test_send_invalid_webxdc() -> Result<()> {
+        let t = TestContext::new_alice().await;
+        let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "foo").await?;
+
+        // sending invalid .xdc as file is possible, but must not result in Viewtype::Webxdc
+        let mut instance = create_webxdc_instance(
+            &t,
+            "invalid-no-zip-but-7z.xdc",
+            include_bytes!("../test-data/webxdc/invalid-no-zip-but-7z.xdc"),
+        )
+        .await?;
+        let instance_id = send_msg(&t, chat_id, &mut instance).await?;
+        assert_eq!(instance.viewtype, Viewtype::File);
+        let test = Message::load_from_db(&t, instance_id).await?;
+        assert_eq!(test.viewtype, Viewtype::File);
+
+        // sending invalid .xdc as Viewtype::Webxdc should fail already on sending
+        let file = t.get_blobdir().join("invalid2.xdc");
+        File::create(&file)
+            .await?
+            .write_all(include_bytes!(
+                "../test-data/webxdc/invalid-no-zip-but-7z.xdc"
+            ))
+            .await?;
+        let mut instance = Message::new(Viewtype::Webxdc);
+        instance.set_file(file.to_str().unwrap(), None);
+        assert!(send_msg(&t, chat_id, &mut instance).await.is_err());
+
+        Ok(())
+    }
+
     #[async_std::test]
     async fn test_forward_webxdc_instance() -> Result<()> {
         let t = TestContext::new_alice().await;