check-if-email-exists icon indicating copy to clipboard operation
check-if-email-exists copied to clipboard

Make the syntax checker respect the RFCs

Open dertuxmalwieder opened this issue 4 years ago • 5 comments

According to the source code, foo@bar is assumed to be an invalid e-mail address. This is a rather short approach at validating e-mail addresses, as foo@bar is totally legit inside your intranet according to the RFCs.

My e-mail address validation library (libvldmail) covers a few test cases which should provide adequate guidelines. Alternatively, you could use my library over FFI and be done with it.

Or, a third possibility, you’ll stick with your rather incomplete (does it have an @ and a dot?) implementation and just ignore my suggestion. After all, it’s your project. ;-)

dertuxmalwieder avatar Mar 26 '21 00:03 dertuxmalwieder

Thanks @dertuxmalwieder! You're 100% right.

In practice, Reacher's customers never validate uncommon addresses likes foo@bar, so it's a non-issue for Reacher (the business). However, for the correctness of the software, yes, it should be fixed.

amaury1093 avatar Mar 26 '21 09:03 amaury1093

I don't think RFC compliance is that important. I just think that the requirement to have a dot should be relaxed, at least when the part after the @ is a valid registered TLD

eligrey avatar Jul 02 '21 17:07 eligrey

Or an IP address which is also valid. Or an intranet mail address. Or...

You see, it's not that easy.

dertuxmalwieder avatar Jul 02 '21 17:07 dertuxmalwieder

I personally won't work on this, as I do not need it for Reacher's use case (100% of emails being validated on Reacher are "normal").

If someone wants to pick this up, feel free. Just small note: I would put this behind a configuration, like input.syntax_respect_rfc = true. On https://reacher.email, I would still like to flag foo@bar as invalid.

amaury1093 avatar Aug 15 '22 09:08 amaury1093

I spent a few hours trying to draft code for this, and, while having a working version was relatively easy, making it entirely optional seems impossible to me:

  1. Add libvldmail as a Git submodule for core:
cd core ; mkdir libs
git submodule add https://github.com/dertuxmalwieder/libvldmail libs/libvldmail
  1. Compile libvldmail as a part of the build process of core: Add the cc crate ...
cd ..
ed core/Cargo.toml
# add this:
[build-dependencies]
cc = { version = "1.0", features = ["parallel"] }
  1. ... and write a build script as core/build.rs (cc needs this exact path):
fn main() {
    cc::Build::new()
        .file("libs/libvldmail/src/vldmail.c")
        .compile("vldmail");
}
  1. Use the validate_email code wherever needed (I'd probably use check_email()):
use std::os::raw::c_char;

// valid_mail_t struct:
#[repr(C)]
struct valid_mail_t {
    status: i8,
    message: *const c_char,
}

// FFI:
extern "C" fn validate_email(adr: &'str) -> valid_mail_t;

// Use it - in theory, depending on a CONF.* variable:
unsafe {
    let is_valid_rfc = validate_email(email);
    // ... now we can use the return value in our CheckEmailOutput() stats, for example.
}

Something like that.

Problems with this approach:

  • Depending on libvldmail is not optional, it will always have to be pulled in and compiled and linked.
  • unsafe { } is required, with all of its drawbacks.
  • I have not figured out a sane way to integrate libvldmail into the output statistics without refactoring the whole statistics part and without making the library mandatory.

But maybe, someone who knows Rust better than I do could pick up the pieces. :-)

dertuxmalwieder avatar Aug 15 '22 20:08 dertuxmalwieder