Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #1474 "Sending message to contact with < or > in Recipient gets treated as "Sent" but is not received" #1476

Merged
merged 5 commits into from
May 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ bitflags = "1.1.0"
debug_stub_derive = "0.3.0"
sanitize-filename = "0.2.1"
stop-token = { version = "0.1.1", features = ["unstable"] }
mailparse = "0.12.0"
mailparse = "0.12.1"
encoded-words = { git = "https://github.com/async-email/encoded-words", branch="master" }
native-tls = "0.2.3"
image = { version = "0.22.4", default-features=false, features = ["gif_codec", "jpeg", "ico", "png_codec", "pnm", "webp", "bmp"] }
Expand Down
72 changes: 72 additions & 0 deletions src/contact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
use async_std::path::PathBuf;
use deltachat_derive::*;
use itertools::Itertools;
use lazy_static::lazy_static;
use regex::Regex;

use crate::aheader::EncryptPreference;
use crate::chat::ChatId;
Expand Down Expand Up @@ -238,6 +240,8 @@ impl Contact {
"Cannot create contact with empty address"
);

let (name, addr) = sanitize_name_and_addr(name, addr);

let (contact_id, sth_modified) =
Contact::add_or_lookup(context, name, addr, Origin::ManuallyCreated).await?;
let blocked = Contact::is_blocked_load(context, contact_id).await;
Expand Down Expand Up @@ -1028,6 +1032,24 @@ pub fn addr_normalize(addr: &str) -> &str {
norm
}

fn sanitize_name_and_addr(name: impl AsRef<str>, addr: impl AsRef<str>) -> (String, String) {
lazy_static! {
static ref ADDR_WITH_NAME_REGEX: Regex = Regex::new("(.*)<(.*)>").unwrap();
}
if let Some(captures) = ADDR_WITH_NAME_REGEX.captures(addr.as_ref()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use slice pattern to avoid indexing captures and ensuring that there are exactly 2 groups captured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shortest way I found to do this was:

    if let Some(captures) = ADDR_WITH_NAME_REGEX.captures(addr.as_ref()) {
        if let [n, a] = &captures.iter().collect::<Vec<_>>()[..] {
...

because

  • it is not that easy to build a slice from a Captures struct
  • a slice is not owned, so I can't use map but need the two ifs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I do not think this is worth it because it introduces quite some complexity (unless there is a way to express this shorter I did not find)

(
if name.as_ref().is_empty() {
Hocuri marked this conversation as resolved.
Show resolved Hide resolved
normalize_name(&captures[1])
} else {
name.as_ref().to_string()
},
captures[2].to_string(),
)
} else {
(name.as_ref().to_string(), addr.as_ref().to_string())
}
}

async fn set_block_contact(context: &Context, contact_id: u32, new_blocking: bool) {
if contact_id <= DC_CONTACT_ID_LAST_SPECIAL {
return;
Expand Down Expand Up @@ -1202,6 +1224,10 @@ mod tests {
assert_eq!(may_be_valid_addr("u@d.tt"), true);
assert_eq!(may_be_valid_addr("u@.tt"), false);
assert_eq!(may_be_valid_addr("@d.tt"), false);
assert_eq!(may_be_valid_addr("<da@d.tt"), false);
assert_eq!(may_be_valid_addr("sk <@d.tt>"), false);
assert_eq!(may_be_valid_addr("as@sd.de>"), false);
assert_eq!(may_be_valid_addr("ask dkl@dd.tt"), false);
}

#[test]
Expand Down Expand Up @@ -1528,4 +1554,50 @@ mod tests {
assert!(addr_cmp(" aa@aa.ORG ", "AA@AA.ORG"));
assert!(addr_cmp(" mailto:AA@AA.ORG", "Aa@Aa.orG"));
}

#[async_std::test]
async fn test_name_in_address() {
let t = dummy_context().await;

let contact_id = Contact::create(&t.ctx, "", "<dave@example.org>")
.await
.unwrap();
let contact = Contact::load_from_db(&t.ctx, contact_id).await.unwrap();
assert_eq!(contact.get_name(), "");
assert_eq!(contact.get_addr(), "dave@example.org");

let contact_id = Contact::create(&t.ctx, "", "Mueller, Dave <dave@example.org>")
.await
.unwrap();
let contact = Contact::load_from_db(&t.ctx, contact_id).await.unwrap();
assert_eq!(contact.get_name(), "Dave Mueller");
assert_eq!(contact.get_addr(), "dave@example.org");
Hocuri marked this conversation as resolved.
Show resolved Hide resolved

let contact_id = Contact::create(&t.ctx, "name1", "name2 <dave@example.org>")
.await
.unwrap();
let contact = Contact::load_from_db(&t.ctx, contact_id).await.unwrap();
assert_eq!(contact.get_name(), "name1");
assert_eq!(contact.get_addr(), "dave@example.org");

assert!(Contact::create(&t.ctx, "", "<dskjfdslk@sadklj.dk")
.await
.is_err());
assert!(Contact::create(&t.ctx, "", "<dskjf>dslk@sadklj.dk>")
.await
.is_err());
assert!(Contact::create(&t.ctx, "", "dskjfdslksadklj.dk")
.await
.is_err());
assert!(Contact::create(&t.ctx, "", "dskjfdslk@sadklj.dk>")
.await
.is_err());
assert!(Contact::create(&t.ctx, "", "dskjf@dslk@sadkljdk")
.await
.is_err());
assert!(Contact::create(&t.ctx, "", "dskjf dslk@d.e").await.is_err());
assert!(Contact::create(&t.ctx, "", "<dskjf dslk@sadklj.dk")
.await
.is_err());
}
}
26 changes: 12 additions & 14 deletions src/dc_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,15 +482,6 @@ pub struct InvalidEmailError {
addr: String,
}

impl InvalidEmailError {
fn new(msg: impl Into<String>, addr: impl Into<String>) -> InvalidEmailError {
InvalidEmailError {
message: msg.into(),
addr: addr.into(),
}
}
}

/// Very simple email address wrapper.
///
/// Represents an email address, right now just the `name@domain` portion.
Expand Down Expand Up @@ -530,17 +521,24 @@ impl FromStr for EmailAddress {

/// Performs a dead-simple parse of an email address.
fn from_str(input: &str) -> Result<EmailAddress, InvalidEmailError> {
if input.is_empty() {
return Err(InvalidEmailError::new("empty string is not valid", input));
}
let parts: Vec<&str> = input.rsplitn(2, '@').collect();

let err = |msg: &str| {
Err(InvalidEmailError {
message: msg.to_string(),
addr: input.to_string(),
})
};
if input.is_empty() {
return err("empty string is not valid");
Hocuri marked this conversation as resolved.
Show resolved Hide resolved
}
let parts: Vec<&str> = input.rsplitn(2, '@').collect();

if input
.chars()
.any(|c| c.is_whitespace() || c == '<' || c == '>')
{
return err("Email must not contain whitespaces, '>' or '<'");
}

match &parts[..] {
[domain, local] => {
if local.is_empty() {
Expand Down