Extend email format tests

[ChALkeR]

Before this, a simple /@/ check passed this format test.

This now extends the test a bit more, to check that local part is checked at least for something.

Symbol check + dot position check is a good indication here, because:

1. It's common and doesn't need implementing more complex logic (quotes etc)
2. Dot is the only symbol which validity depends on the position in the local part

This should trigger most of the naive implementations (e.g. /@/) and is still not hard to implement.

JSON Schema uses RFC 5322.

Refs: https://tools.ietf.org/html/rfc5322#page-17
Refs: https://tools.ietf.org/html/rfc5322#page-12

Also, some explanation for the two dots (with links to same RFC pages): https://serverfault.com/questions/395766/are-two-periods-allowed-in-the-local-part-of-an-email-address

[ChALkeR]

ajv should pass all of these.

Whoops. It passes all the dot checks correctly but fails root@localhost for some reason.
Strange, require('ajv')().compile({format:'email'})('root@localhost') works but npm t in this repo fails on that.

Upd: ah, because of require('ajv')({format:'full'}).compile({format:'email'})('root@localhost')

Upd: I just removed root@localhost for now due to not being sure if that's intended behavior or not.

[karenetheridge]

The library I checked with does not think the last two tests (space in domain; symbol in domain) are correct with respect to RFC 2822.

[ChALkeR]

The library I checked with does not think the last two tests (space in domain; symbol in domain) are correct with respect to RFC 2822.

But they are listed there with valid: false, so that's as it should be?

[karenetheridge]

No, I mean that the library thinks those two cases are valid emails.

[ChALkeR]

Ah, I misread your comment, sorry.

Reading RFC 5322, it does indeed seem that & in the domain part is allowed (which is strange, I would say).
Space shouldn't be allowed though.

But I removed both to be on the safe side, because I'm not entirely sure there, domain part of the spec seems trickier than I initially thought.

[Julian]

Before this, a simple /@/ check passed this format test.

FWIW this is exactly how my implementation implements the email checker as well :) (and will continue to do so) -- though as I've said in #253 I'm fine with other folks adding more tests here obviously, those of us who don't want to implement more strict email format verification will just skip the new ones.

[ssilverman]

My two cents: emails, much more so than hostnames, are hard to parse well, and I wouldn't include any false tests here. Yes, the tests you have here are technically valid, and yes, they follow the RFCs, but email is notoriously "flexible".

Note that even checking for the presence of a "@" is technically wrong because the escaped form, "\@" is allowed inside the local-part (and actually allowed in any quoted parts of that local part, non-escaped, "Abc@def"@example.com, for example. See: RFC 3696, Section 3). And don't get me started on the dot-separated "quoted-string OR atom" form in obs-local-part.

[ChALkeR]

@ssilverman I don't think that unquoted \@ is allowed per RFC 5322, which the spec uses?
And this purposely excludes any tests for quoted strings.

wouldn't include any false tests here

Then email format should be removed from the spec alltogether, if users can't assume that it will return false on at least something. I.e. if we can't assume that email validation does at least something -- why is that even included?

The intent of this PR is to check if any minimal email validation is present apart from a /@/ check.
If there are some easier tests that these -- I am fine with replacing these with those :-).

[ssilverman]

I don't disagree that test sets should contain false tests. I just think they make life complicated specifically for emails. This is just my opinion and two cents.

Per the link in my comment (RFC 3696, Section 3), the following emails are valid (I'm repeating the link for posterity and for future readers):

1. Unquoted form: Abc\@[email protected]
2. Quoted form: "Abc@def"@example.com

My point is that whether that's allowed in RFC 5322 or not, you can bet someone uses them somewhere. :)

Emails get very messy to parse "properly" and I personally don't see much value in even writing strict tests for them. The domain parts, maybe, but not the local part (before the '@'). Just to emphasize: I don't disagree that these tests work and that it's often a good idea to test things about a grammar, but emails are a special kind of difficult when you see what's out in the wild.

Emails are a case where "best practices" sometimes disagree with the specs and various grammars.

The tests will test the RFC 5322 grammar, sure, but it's possible for "real emails" to violate this. This is simply my opinion, and I won't object strongly to the tests being included.

[ssilverman]

I'll add, regarding backslash-escapes from RFC 5322, just to illustrate that it gets messy:

1. local-part can contain obs-local-part which can contain words.
2. words are either atoms or quoted-strings.
3. quoted-strings may contain qtext or quoted_pairs.
4. And, if you continue going down into the grammar, you'll find that VCHARs include the '@' character. So now, you can have backslash-escaped '@' characters inside quoted strings.

To me, I interpret that RFC 5322 flips what are valid emails with backslash-escapes. They can appear inside quoted strings, just like RFC 2822 allows (heck, I don't even think that they're allowed outside quoted strings there, unlike what RFC 3696 mentions). (And yes, I know the tests in this PR don't include quoted strings, as you mention above, I'm just continuing the description of my '@' example, for future readers.)

Grammars are certainly "fun"! :)

[ssilverman]

For posterity, here was the regex I used until recently: ^[^@]+@[^@]+$.
Here's the regex I use now: ^.*[^\\]@[^@]+$
Double-yuck. 🤢

[ssilverman]

If others are good with it, I'm good with it.