I'm a self-proclaimed expert on the topic of email, specifically MIME, IMAP, SMTP, and POP3. I don't proclaim myself to be an expert on much, but email is something that maybe 1 or 2 dozen people in the world could probably get away with saying they know more than I do and actually back it up. I've got a lot of experience writing email software over the past 15 years and rarely do I come across mail software that does things better than I've done them. I'm also a critic of mail software design and implementation.
My latest endeavors in the email space are MimeKit and MailKit, both of which are open source and available on GitHub for your perusal should you doubt my expertise.
My point is: I think my review carries some weight, or I wouldn't be writing this.
Is that egotistical of me? Maybe a little.
I was actually just fixing a bug in MimeKit earlier and when I went to go examine Mono's System.Net.Mail.MailMessage implementation in order to figure out what the problem was with my System.Net.Mail.MailMessage to MimeKit.MimeMessage conversion, I thought, "hey, wait a minute... didn't Microsoft just recently release their BCL source code?" So I ended up taking a look and pretty quickly confirmed my suspicions and was able to fix the bug.
When I begin looking at the source code for another mail library, I can't help but critique what I find.
MailAddress and MailAddressCollection
Parsing email addresses is probably the hardest thing to get right. It's what I would say makes or breaks a library (literally). To a casual onlooker, parsing email addresses probably seems like a trivial problem. "Just String.Split() on comma and then look for those angle bracket thingies and you're done, right?" Oh God, oh God, make the hurting stop. I need to stop here before I go into a long rant about this...
Okay, I'm back. Blood pressure has subsided.
Looking at MailAddressParser.cs (the internal parser used by MailAddressCollection), I'm actually pleasantly surprised. It actually looks pretty decent and I can tell that a lot of thought and care went into it. They actually use a tokenizer approach. Interestingly, they parse the string in reverse which is a pretty good idea, I must say. This approach probably helps simplify the parser logic a bit because parsing forward makes it difficult to know what the tokens belong to (is it the name token? or is it the local-part of an addr-spec? hard to know until I consume a few more tokens...).
For example, consider the following BNF grammar:
address = mailbox / group
mailbox = name-addr / addr-spec
name-addr = [display-name] angle-addr
angle-addr = [CFWS] "<" addr-spec ">" [CFWS] / obs-angle-addr
group = display-name ":" [mailbox-list / CFWS] ";"
[CFWS]
display-name = phrase
word = atom / quoted-string
phrase = 1*word / obs-phrase
addr-spec = local-part "@" domain
local-part = dot-atom / quoted-string / obs-local-part
domain = dot-atom / domain-literal / obs-domain
obs-local-part = word *("." word)
Now consider the following email address: "Joe Example" <joe@example.com>
The first token you read will be "Joe Example" and you might think that that token indicates that it is the display name, but it doesn't. All you know is that you've got a 'quoted-string' token. A 'quoted-string' can be part of a 'phrase' or it can be (a part of) the 'local-part' of the address itself. You must read at least 1 more token before you'll be able to figure out what it actually is ('obs-local-part' makes things slightly more difficult). In this case, you'll get a '<' which indicates the start of an 'angle-addr', allowing you to assume that the 'quoted-string' you just got is indeed the 'display-name'.
If, however, you parse the address in reverse, things become a little simpler because you know immediately what to expect the next token to be a part of.
That's pretty cool. Kudos to the Microsoft engineers for thinking up this strategy.
Unfortunately, the parser does not handle the 'group' address type. I'll let this slide, however, partly because I'm still impressed by the approach the address parser took and also because I realize that System.Net.Mail is meant for creating and sending new messages, not parsing existing messages from the wild.
Okay, so how well does it serialize MailAddress?
Ugh. You know that face you make when you just see a guy get kicked in the nuts? Yea, that's the face I made when I saw line #227:
encodedAddress = String.Format(CultureInfo.InvariantCulture, "\"{0}\"", this.displayName);
The problem with the above code (and I'll soon be submitting a bug report about this) is that the displayName string might have embedded double quotes in it. You can't just surround it with quotes and expect it to work. This is the same mistake all those programmers make that allow SQL-injection attacks.
For an example of how this should be done, see MimeKit's MimeUtils.Quote() method.
I had such high hopes... at least this is a fairly simple bug to fix. I'll probably just offer them a patch.
ContentType and ContentDisposition
Their parser is decent but it doesn't handle rfc2231 encoded parameter values, so I'm not overly impressed. It'll get the job done for simple name="value" parameter syntax, though, and it will decode the values encoded with the rfc2047 encoding scheme (which is not the right way to encode values, but it is common enough that any serious parser should handle it). The code is also pretty clean and uses a tokenizer approach, so that's a plus. I guess since this isn't really meant as a full-blown MIME parser, they can get away with this and not have it be a big deal. Fair enough.
Serialization, unsurprisingly, leaves a lot to be desired. Parameter values are, as I expected, encoded using rfc2047 syntax rather than the IETF standard rfc2231 syntax. I suppose that you could argue that this is for compatibility, but really it's just perpetuating bad practices. It also means that it can't properly fold long parameter values because the encoded value just becomes one big long encoded-word token. Yuck.
Base64
Amusingly, Microsoft does not use their Convert.FromBase64() decoder to decode base64 in their System.Net.Mail implementation. I point this out mostly because it is the single most common problem users have with every one of the Open Source .NET mail libraries out there (other than MimeKit, of course) because Convert.FromBase64() relies on the data not having any line breaks, white space, etc in the input stream.
This should serve as a big hint to you guys writing your own .NET email libraries not to use Convert.FromBase64() ;-)
They use unsafe pointers, just like I do in MimeKit, but I'm not sure how their performance compares to MimeKit's yet. They do use a state machine, though, so rock on.
I approve this base64 encoder/decoder implementation.
SmtpClient
One thing they do which is pretty cool is connection pooling. This is probably a pretty decent win for the types of things developers usually use System.Net.Mail's SmtpClient for (spam, anyone?).
The SASL AUTH mechanisms that they seem to support are NTLM, GSSAPI, LOGIN and WDIGEST (which apparently is some sort of IIS-specific authentication mechanism that I had never heard of until now). For those that were curious which SASL mechanisms SmtpClient supported, well, now you know.
The code is a bit hard to follow for someone not familiar with the codebase (not nearly as easy reading as the address or content-type parsers, I'm afraid), but it seems fairly well designed.
It does not appear to support PIPELINING or BINARYMIME like MailKit does, though. So, yay! Win for MailKit ;-)
They do support SMTPUTF8, so that's good.
It seems that if you set client.EnableSsl to true, it will also try STARTTLS if it isn't able to connect on the SSL port. I wasn't sure if it did that or not before, so this was something I was personally interested in knowing.
Hopefully my SmtpClient implementation review isn't too disappointing. I just don't know what to say about it, really. It's a pretty straight-forward send-command-wait-for-reply implementation and SMTP is pretty dead simple.
Conclusion
Overall the bits I was interested in were better than I expected they'd be. The parsers were pretty good (although incomplete) and the serializers were "good enough" for normal use.
Of course, it's not as good as MimeKit, but let's be honest, MimeKit sets the bar pretty high ;-)
No comments:
Post a Comment