andyedinborough / aenetmail

C# POP/IMAP Mail Client
370 stars 153 forks source link

unchecked error returns leading to problems #98

Closed smr888 closed 11 years ago

smr888 commented 12 years ago

MailMessage, method "ParseMime":

Approx line 153, variable data filled from "reader.ReadLine()". checks are made for data being null or not in the loop control variables, leading one to believe that ReadLine() can return null. Yet at 162 and other places in this method no check for null-ness is done at all, or a check is made against String.Empty.

The reasons I'm here . . .

Utilities, method "ReadLine":

Line 22 b = (byte) stream.ReadByte(); is executed, but no check is made for end of stream (value -1, or 255)

I am running into endless loops where 255 is returned.

TextClient, method "GetResponse":

same thing,

Line 106 b = (byte)_Stream.ReadByte();

no check is ever done for end of stream. The while (true), in some cases, simply runs forever.

Call looks something like this:

var msgs = imap.SearchMessages(SearchCondition.Undeleted().And(SearchCondition.Subject("find in subject").And(SearchCondition.Body("find in body"))); foreach (System.Lazy message in msgs) { MailMessage msg = message.Value;

that last statement never returns when the -1s are encountered.

I tried jamming in some checks but it is difficult to tell what sort of return value the code expects when no data is found (see my first comment).

VS 10, .Net 4, if that is of interest.

gitcorvid commented 12 years ago

I'm seeing the same choking with an attachment. I tried putting the check in for 255 and a check for empty strings which stopped the infinite loop but punted the problem downstream to line 159 in Utilities:

      data[n] = (byte)int.Parse(value.Substring(i + 1, 2), NumberStyles.HexNumber);

This boat is dead in the water. Please send reinforcements.

537mfb commented 11 years ago

Am seeing this too with the latest version - any fixes yet? My workaround was to encapsulate my getMessageHeader line in a try catch and discard the message if it throws an error - but frankly that's not good. God know how many messages i can loose this way.

smr888 commented 11 years ago

Giving up on this library. WADR, and I really do appreciate the effort you made on these libraries, if you're going to talk in your promo about how all the other libraries you found when you had this need were crap and you rolled your own obviously much better one, it ought not to have fundamental errors in it like unchecked return values -- and when they are pointed out, some work ought to be done to fix them.

As it is, I'm off to Mail.dll. For a savings of $199 it's not worth my time dealing with the usual open source amateur hour.

Sorry to be so harsh, but how you could ignore this bug is beyond me. I receive a large number of e-mails that this library bites the weenie on, and that is not ok.

andyedinborough commented 11 years ago

smr888, dude relax. I'm not ignoring anything. I've got a full-time job and haven't had any time to put into this. I'm only have time now because I'm sick and my wife made me go to bed.

It's called open-source, so you can fork it, fix it, submit a pull-request, and everyone wins.

When was the last time you pulled the latest source? As far as I'm aware, this bug was fixed in 89adc9c53b5431b248e8e1031f3db7f253d78031, about 2 months ago.

smr888 commented 11 years ago

I relaxed for five months! Then gave up. I guess as a business user I just don't have that kind of time to wait, nor to try to fix bugs myself (although I did take a look, but gave up when I could not determine whether it was acceptable for ReadLine() to return null or not, or whether it had to return "", or what, as mentioned in my first post).