Merge pull request #683 from deltachat/outlook_none

Fix and refactor Outlook autodiscovery

src/configure/auto_mozilla.rs

@@ -1,10 +1,8 @@
-use libc::free;
 use quick_xml;
 use quick_xml::events::{BytesEnd, BytesStart, BytesText};
 
 use crate::constants::*;
 use crate::context::Context;
-use crate::dc_tools::*;
 use crate::login_param::LoginParam;
 
 use super::read_autoconf_file;
@@ -24,26 +22,22 @@ struct moz_autoconfigure_t<'a> {
     pub tag_config: libc::c_int,
 }
 
-pub unsafe fn moz_autoconfigure(
+pub fn moz_autoconfigure(
     context: &Context,
     url: &str,
     param_in: &LoginParam,
 ) -> Option<LoginParam> {
-    let xml_raw = read_autoconf_file(context, url);
-    if xml_raw.is_null() {
-        return None;
-    }
+    let xml_raw = read_autoconf_file(context, url)?;
 
     // Split address into local part and domain part.
     let p = param_in.addr.find("@");
     if p.is_none() {
-        free(xml_raw as *mut libc::c_void);
         return None;
     }
     let (in_emaillocalpart, in_emaildomain) = param_in.addr.split_at(p.unwrap());
     let in_emaildomain = &in_emaildomain[1..];
 
-    let mut reader = quick_xml::Reader::from_str(as_str(xml_raw));
+    let mut reader = quick_xml::Reader::from_str(&xml_raw);
     reader.trim_text(true);
 
     let mut buf = Vec::new();
@@ -88,11 +82,9 @@ pub unsafe fn moz_autoconfigure(
     {
         let r = moz_ac.out.to_string();
         warn!(context, "Bad or incomplete autoconfig: {}", r,);
-        free(xml_raw as *mut libc::c_void);
         return None;
     }
 
-    free(xml_raw as *mut libc::c_void);
     Some(moz_ac.out)
 }
 

src/configure/auto_outlook.rs

@@ -1,212 +1,157 @@
-use std::ptr;
-
-use libc::free;
 use quick_xml;
-use quick_xml::events::{BytesEnd, BytesStart, BytesText};
+use quick_xml::events::BytesEnd;
 
 use crate::constants::*;
 use crate::context::Context;
-use crate::dc_tools::*;
 use crate::login_param::LoginParam;
 
 use super::read_autoconf_file;
-/* ******************************************************************************
- * Outlook's Autodiscover
- ******************************************************************************/
-#[repr(C)]
-struct outlk_autodiscover_t<'a> {
-    pub in_0: &'a LoginParam,
+
+/// Outlook's Autodiscover
+struct OutlookAutodiscover {
     pub out: LoginParam,
-    pub out_imap_set: libc::c_int,
-    pub out_smtp_set: libc::c_int,
-    pub tag_config: libc::c_int,
-    pub config: [*mut libc::c_char; 6],
+    pub out_imap_set: bool,
+    pub out_smtp_set: bool,
+    pub config_type: Option<String>,
+    pub config_server: String,
+    pub config_port: i32,
+    pub config_ssl: String,
+    pub config_redirecturl: Option<String>,
 }
 
-pub unsafe fn outlk_autodiscover(
+pub fn outlk_autodiscover(
     context: &Context,
-    url__: &str,
-    param_in: &LoginParam,
+    url: &str,
+    _param_in: &LoginParam,
 ) -> Option<LoginParam> {
-    let mut xml_raw: *mut libc::c_char = ptr::null_mut();
-    let mut url = url__.strdup();
-    let mut outlk_ad = outlk_autodiscover_t {
-        in_0: param_in,
-        out: LoginParam::new(),
-        out_imap_set: 0,
-        out_smtp_set: 0,
-        tag_config: 0,
-        config: [ptr::null_mut(); 6],
-    };
-    let ok_to_continue;
-    let mut i = 0;
-    loop {
-        if i >= 10 {
-            ok_to_continue = true;
-            break;
-        }
-        libc::memset(
-            &mut outlk_ad as *mut outlk_autodiscover_t as *mut libc::c_void,
-            0,
-            ::std::mem::size_of::<outlk_autodiscover_t>(),
-        );
-        xml_raw = read_autoconf_file(context, as_str(url));
-        if xml_raw.is_null() {
-            ok_to_continue = false;
-            break;
-        }
+    let mut url = url.to_string();
+    /* Follow up to 10 xml-redirects (http-redirects are followed in read_autoconf_file() */
+    for _i in 0..10 {
+        let mut outlk_ad = OutlookAutodiscover {
+            out: LoginParam::new(),
+            out_imap_set: false,
+            out_smtp_set: false,
+            config_type: None,
+            config_server: String::new(),
+            config_port: 0,
+            config_ssl: String::new(),
+            config_redirecturl: None,
+        };
 
-        let mut reader = quick_xml::Reader::from_str(as_str(xml_raw));
-        reader.trim_text(true);
+        if let Some(xml_raw) = read_autoconf_file(context, &url) {
+            let mut reader = quick_xml::Reader::from_str(&xml_raw);
+            reader.trim_text(true);
 
-        let mut buf = Vec::new();
+            let mut buf = Vec::new();
 
-        loop {
-            match reader.read_event(&mut buf) {
-                Ok(quick_xml::events::Event::Start(ref e)) => {
-                    outlk_autodiscover_starttag_cb(e, &mut outlk_ad)
+            let mut current_tag: Option<String> = None;
+
+            loop {
+                match reader.read_event(&mut buf) {
+                    Ok(quick_xml::events::Event::Start(ref e)) => {
+                        let tag = String::from_utf8_lossy(e.name()).trim().to_lowercase();
+
+                        if tag == "protocol" {
+                            outlk_ad.config_type = None;
+                            outlk_ad.config_server = String::new();
+                            outlk_ad.config_port = 0;
+                            outlk_ad.config_ssl = String::new();
+                            outlk_ad.config_redirecturl = None;
+
+                            current_tag = None;
+                        } else {
+                            current_tag = Some(tag);
+                        }
+                    }
+                    Ok(quick_xml::events::Event::End(ref e)) => {
+                        outlk_autodiscover_endtag_cb(e, &mut outlk_ad);
+                        current_tag = None;
+                    }
+                    Ok(quick_xml::events::Event::Text(ref e)) => {
+                        let val = e.unescape_and_decode(&reader).unwrap_or_default();
+
+                        if let Some(ref tag) = current_tag {
+                            match tag.as_str() {
+                                "type" => outlk_ad.config_type = Some(val.trim().to_string()),
+                                "server" => outlk_ad.config_server = val.trim().to_string(),
+                                "port" => {
+                                    outlk_ad.config_port = val.trim().parse().unwrap_or_default()
+                                }
+                                "ssl" => outlk_ad.config_ssl = val.trim().to_string(),
+                                "redirecturl" => {
+                                    outlk_ad.config_redirecturl = Some(val.trim().to_string())
+                                }
+                                _ => {}
+                            };
+                        }
+                    }
+                    Err(e) => {
+                        error!(
+                            context,
+                            "Configure xml: Error at position {}: {:?}",
+                            reader.buffer_position(),
+                            e
+                        );
+                    }
+                    Ok(quick_xml::events::Event::Eof) => break,
+                    _ => (),
                 }
-                Ok(quick_xml::events::Event::End(ref e)) => {
-                    outlk_autodiscover_endtag_cb(e, &mut outlk_ad)
-                }
-                Ok(quick_xml::events::Event::Text(ref e)) => {
-                    outlk_autodiscover_text_cb(e, &mut outlk_ad, &reader)
-                }
-                Err(e) => {
-                    error!(
-                        context,
-                        "Configure xml: Error at position {}: {:?}",
-                        reader.buffer_position(),
-                        e
-                    );
-                }
-                Ok(quick_xml::events::Event::Eof) => break,
-                _ => (),
+                buf.clear();
             }
-            buf.clear();
-        }
-
-        if !(!outlk_ad.config[5].is_null()
-            && 0 != *outlk_ad.config[5usize].offset(0isize) as libc::c_int)
-        {
-            ok_to_continue = true;
-            break;
-        }
-        free(url as *mut libc::c_void);
-        url = dc_strdup(outlk_ad.config[5usize]);
-
-        outlk_clean_config(&mut outlk_ad);
-        free(xml_raw as *mut libc::c_void);
-        xml_raw = ptr::null_mut();
-        i += 1;
-    }
-
-    if ok_to_continue {
-        if outlk_ad.out.mail_server.is_empty()
-            || outlk_ad.out.mail_port == 0
-            || outlk_ad.out.send_server.is_empty()
-            || outlk_ad.out.send_port == 0
-        {
-            let r = outlk_ad.out.to_string();
-            warn!(context, "Bad or incomplete autoconfig: {}", r,);
-            free(url as *mut libc::c_void);
-            free(xml_raw as *mut libc::c_void);
-            outlk_clean_config(&mut outlk_ad);
 
+            // XML redirect via redirecturl
+            if outlk_ad.config_redirecturl.is_none()
+                || outlk_ad.config_redirecturl.as_ref().unwrap().is_empty()
+            {
+                if outlk_ad.out.mail_server.is_empty()
+                    || outlk_ad.out.mail_port == 0
+                    || outlk_ad.out.send_server.is_empty()
+                    || outlk_ad.out.send_port == 0
+                {
+                    let r = outlk_ad.out.to_string();
+                    warn!(context, "Bad or incomplete autoconfig: {}", r,);
+                    return None;
+                }
+                return Some(outlk_ad.out);
+            } else {
+                url = outlk_ad.config_redirecturl.unwrap();
+            }
+        } else {
             return None;
         }
     }
-    free(url as *mut libc::c_void);
-    free(xml_raw as *mut libc::c_void);
-    outlk_clean_config(&mut outlk_ad);
-    Some(outlk_ad.out)
+    None
 }
 
-unsafe fn outlk_clean_config(mut outlk_ad: *mut outlk_autodiscover_t) {
-    for i in 0..6 {
-        free((*outlk_ad).config[i] as *mut libc::c_void);
-        (*outlk_ad).config[i] = ptr::null_mut();
-    }
-}
-
-fn outlk_autodiscover_text_cb<B: std::io::BufRead>(
-    event: &BytesText,
-    outlk_ad: &mut outlk_autodiscover_t,
-    reader: &quick_xml::Reader<B>,
-) {
-    let val = event.unescape_and_decode(reader).unwrap_or_default();
-
-    unsafe {
-        free(outlk_ad.config[outlk_ad.tag_config as usize].cast());
-        outlk_ad.config[outlk_ad.tag_config as usize] = val.trim().strdup();
-    }
-}
-
-unsafe fn outlk_autodiscover_endtag_cb(event: &BytesEnd, outlk_ad: &mut outlk_autodiscover_t) {
+fn outlk_autodiscover_endtag_cb(event: &BytesEnd, outlk_ad: &mut OutlookAutodiscover) {
     let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase();
 
     if tag == "protocol" {
-        if !outlk_ad.config[1].is_null() {
-            let port = dc_atoi_null_is_0(outlk_ad.config[3]);
-            let ssl_on = (!outlk_ad.config[4].is_null()
-                && strcasecmp(
-                    outlk_ad.config[4],
-                    b"on\x00" as *const u8 as *const libc::c_char,
-                ) == 0) as libc::c_int;
-            let ssl_off = (!outlk_ad.config[4].is_null()
-                && strcasecmp(
-                    outlk_ad.config[4],
-                    b"off\x00" as *const u8 as *const libc::c_char,
-                ) == 0) as libc::c_int;
-            if strcasecmp(
-                outlk_ad.config[1],
-                b"imap\x00" as *const u8 as *const libc::c_char,
-            ) == 0
-                && outlk_ad.out_imap_set == 0
-            {
-                outlk_ad.out.mail_server = to_string_lossy(outlk_ad.config[2]);
+        if let Some(type_) = &outlk_ad.config_type {
+            let port = outlk_ad.config_port;
+            let ssl_on = outlk_ad.config_ssl == "on";
+            let ssl_off = outlk_ad.config_ssl == "off";
+            if type_ == "imap" && !outlk_ad.out_imap_set {
+                outlk_ad.out.mail_server =
+                    std::mem::replace(&mut outlk_ad.config_server, String::new());
                 outlk_ad.out.mail_port = port;
-                if 0 != ssl_on {
+                if ssl_on {
                     outlk_ad.out.server_flags |= DC_LP_IMAP_SOCKET_SSL as i32
-                } else if 0 != ssl_off {
+                } else if ssl_off {
                     outlk_ad.out.server_flags |= DC_LP_IMAP_SOCKET_PLAIN as i32
                 }
-                outlk_ad.out_imap_set = 1
-            } else if strcasecmp(
-                outlk_ad.config[1usize],
-                b"smtp\x00" as *const u8 as *const libc::c_char,
-            ) == 0
-                && outlk_ad.out_smtp_set == 0
-            {
-                outlk_ad.out.send_server = to_string_lossy(outlk_ad.config[2]);
-                outlk_ad.out.send_port = port;
-                if 0 != ssl_on {
+                outlk_ad.out_imap_set = true
+            } else if type_ == "smtp" && !outlk_ad.out_smtp_set {
+                outlk_ad.out.send_server =
+                    std::mem::replace(&mut outlk_ad.config_server, String::new());
+                outlk_ad.out.send_port = outlk_ad.config_port;
+                if ssl_on {
                     outlk_ad.out.server_flags |= DC_LP_SMTP_SOCKET_SSL as i32
-                } else if 0 != ssl_off {
+                } else if ssl_off {
                     outlk_ad.out.server_flags |= DC_LP_SMTP_SOCKET_PLAIN as i32
                 }
-                outlk_ad.out_smtp_set = 1
+                outlk_ad.out_smtp_set = true
             }
         }
-        outlk_clean_config(outlk_ad);
     }
-    outlk_ad.tag_config = 0;
-}
-
-fn outlk_autodiscover_starttag_cb(event: &BytesStart, outlk_ad: &mut outlk_autodiscover_t) {
-    let tag = String::from_utf8_lossy(event.name()).trim().to_lowercase();
-
-    if tag == "protocol" {
-        unsafe { outlk_clean_config(outlk_ad) };
-    } else if tag == "type" {
-        outlk_ad.tag_config = 1
-    } else if tag == "server" {
-        outlk_ad.tag_config = 2
-    } else if tag == "port" {
-        outlk_ad.tag_config = 3
-    } else if tag == "ssl" {
-        outlk_ad.tag_config = 4
-    } else if tag == "redirecturl" {
-        outlk_ad.tag_config = 5
-    };
 }

src/configure/mod.rs

@@ -633,7 +633,7 @@ pub fn dc_stop_ongoing_process(context: &Context) {
     };
 }
 
-pub fn read_autoconf_file(context: &Context, url: &str) -> *mut libc::c_char {
+pub fn read_autoconf_file(context: &Context, url: &str) -> Option<String> {
     info!(context, "Testing {} ...", url);
 
     match reqwest::Client::new()
@@ -641,11 +641,11 @@ pub fn read_autoconf_file(context: &Context, url: &str) -> *mut libc::c_char {
         .send()
         .and_then(|mut res| res.text())
     {
-        Ok(res) => unsafe { res.strdup() },
+        Ok(res) => Some(res),
         Err(_err) => {
             info!(context, "Can\'t read file.",);
 
-            std::ptr::null_mut()
+            None
         }
     }
 }

src/dc_tools.rs

@@ -51,14 +51,6 @@ pub unsafe fn dc_strdup(s: *const libc::c_char) -> *mut libc::c_char {
     ret
 }
 
-pub(crate) fn dc_atoi_null_is_0(s: *const libc::c_char) -> libc::c_int {
-    if !s.is_null() {
-        as_str(s).parse().unwrap_or_default()
-    } else {
-        0
-    }
-}
-
 unsafe fn dc_ltrim(buf: *mut libc::c_char) {
     let mut len: libc::size_t;
     let mut cur: *const libc::c_uchar;