240.01 KB, text/plain
1.46 KB, patch
Scott MacGregor: superreview+
|Details | Diff | Splinter Review|
2.08 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007 If i have 100 Mails on my imap server [Mercur 4.2 www.atrium-software.com] and i would like to download these all to my client, mozilla crashs. I talked to the developers of the Mercur and they told me there is a problem by mozilla. he use negative UID's. This looks it came from a unsignet integer who is overruned. Reproducible: Always Steps to Reproduce: 1.copy 100 Mails on an Imap account on Mercure Mailserver v.4.2 2.try to view one of these mails with mozilla 3.Mozilla would maximal download 30 Messages. In no case it download all messages. Actual Results: In some Cases Mozilla try to donload messages till it crashs, sometimes it runns normaly. but not all messages are here. Expected Results: Mey be it is only the Variable of the UID who have to be bigger, like long or double
please attach a imap protocol log: http://mozilla.org/quality/mailnews/mail-troubleshoot.html#imap
Severity: blocker → normal
besides a protocol log, a test account would be useful as well. In the past, negative UID's have worked, I believe.
Status: UNCONFIRMED → NEW
Ever confirmed: true
[tracking and removed ALL CAPS from bug summary]
Summary: CANT DOWNLOAD ALL MESSAGES FROM MY IMAP ACCOUNT [Negative UID] → Can't download all messages from my IMAP account [Negative UID]
I have the same problem with Windows/XP and Mozilla 1.5. Error message is mentioning just an "invalid sequence in UID" right after I'm starting the download of eMails. It even doesn't synchronize one message.
Created attachment 135938 [details] Imap Log file here is a logfile, the problem occure when i would like to receive mails form the account info2sk8shop.ch and specialli if i would see the contents of Erledigt_Roman Thanky a lot
The same for Thunderbird 1.0/Windows, Thunderbird 1.0/Linux and Mozilla 1.7,1.8a3/Linux. Server: * OK MERCUR IMAP4-Server (v4.02.09 Mjk4MzUtNjI3Mi0xOTQ3MA==) for Windows NT ready at Mon, 10 Jan 2005 16:23:04 +0300 Problem in UIDs higher than 0x7fffffff - somewhere in Mozilla code they were treated as "signed int". There are also possible duplicates for this bug https://bugzilla.mozilla.org/show_bug.cgi?id=277905
Just ran into this with Thunderbird 18.104.22.168 on FredBSD with imap-uw-2006j_3,1. imap-uw does checks for negative UDS and you get a Bogus sequence UID error message from imapd. If you look in mailnews/imap/src, there seems to be 2 issues: 1) there are several methods that use atoi() to convert a string UID to an int then assigns it to an unsigned. For most practical things, this is probably not going to matter much - but it should use stroul() instead. 2) there are several methods that use the AppendInt() method to convert an unsigned int 32 UID to a string. In xpcom/string/public/nsTString.h we declare AppendInt() with an unsigned parameter to call the default AppendInt() method with the unsigned int parameter casted to an int. This will cause a UID higher then 0x7fffffff to end up in the string as a negative number which imap-uw do not like. Adding a AppendInt method in xpcom/string/src/nsStringObsolete.cpp that accepts a unsigned int as a parameter, and uses %u instead of %d in the sprintf() statement seems to fix the problem (at least partially). After having done this, thunderbird will now get all the headers, but it will go into a loop once you try to click on a header to read the mail. I am not sure yet what that is all about. I would suggest you increase the priority on this bug to get it fixed. This recommendation is based on the assumption that IMAP UIDs are defined in the protocol to be Unsigned. Prior versions of imap-uw treated UIDs as signed ints, and thus did not complain.....and thus thunderbird and imap-uw did things the same way ...
Found thunderbird to be looping because a couple of methods were using PRInt32 to hold the current UID. Fixed those and now things seems to work just fine. I have changes to 8 files in mailnews/imap/src: nsIMAPBodyShell.cpp, nsImapMailFolder.cpp nsImapProtocol.cpp, nsImapServerResponseParser.cpp nsImapService.cpp nsImapUndoTxn.cpp nsImapUrl.cpp and nsImapUtils.cpp. I also went back and explicitly called AppendUInt() instead of relying on overloading AppendInt().
I've been discussing this issue with Mark Crispin, the developer of uw-imapd. According to RFC 3501, it's definitely supposed to be an unsigned integer, but both Apple Mail and Horde IMP have the same bug. I only figured out what was going on because Thunderbird actually reports an error, while both other clients just silently ignore the messages. If a spammer sends a message with a forged X-UID header, and you're using mbox format which uses an X-UID header to track UIDs, imapd will be tricked into thinking that using a UID above 2^31 is a good idea, and all subsequent messages will be numbered sequentially above that. I'm working on configuring my MTA to strip out X-UID headers, but obviously there are other situations in which one might encounter UIDs above 2^31.
this bug is fixed in Thunderbird, at least on the trunk, but also in 22.214.171.124, I believe, thx to Lars. So I believe this bug is a dup of the bug that was fixed.
I'm no longer seeing the error message I was getting before: "Bogus sequence in UID FETCH: Syntax error in sequence." But messages with a UID above 2^31 still aren't showing up for me, in 126.96.36.199. Any idea why?
Hi Guys, I just compiled and installed 188.8.131.52, and the "UID FETCH:" error that I originally ran into seems to be fixed as David reported in #10 above. Unfortunately, there still seems to be something left that has not yet been changed to Unsigned. If I click on an e-mail to mark it junk, I get 2 popups saying: "The current command did not succeed. The mail server responded: Bogus sequence in UID STORE: Syntax error in sequence." I still have a debug version of Imapd running where I added some debug statements, and it is definitively seeing negative UIDs in the request. I do not see any X-UID: headers in the message source. Applying my old patches this problem seemd to go away when I fixed the AppendInt() to be AppendUInt() in nsImapUtils.cpp. I did so everywhere we tried to append a PRUint32. nsBuildImapMessageUIR(), AllocateImapUidString(). I also changed the snprintf() in AppendUid() to be a %lu instead of a %u. Lastly I changed curToken and saveStartToken in ParseUidString to be of PRUint32 type instead of int32. If anyone is interested I can provide the diffs I have applied to 184.108.40.206. It will have to wait until after Thanksgiving though :-) Thanks for looking into this issue! Lars
the patch in bug 277905 was landed on the trunk and 2.0.0.x branch - I'm not sure how that patch needs to be extended for the mark as junk case, but I can have a quick look - it should be a very small change...
I looked at the code on the trunk and on the 2.0.0.x branch - it looks to me like the code that stores the /JUNK keyword uses AllocateImapUidString, which should work fine. I'm pretty sure 220.127.116.11 should have those fixes, so I'm not sure what's going on. Is the 18.104.22.168 source that you see still use PRInt32 in AllocateImapUidString()?
David, I'll send you my current diffs, which some are not required based on the changes you have already made, but that way you can look at them if you want. Faster than me trying to type out what I changed. I don't see a way to attach the diff files here. AllocateImapUidString() does use PRUint32, but it still uses AppendInt, where it really should be using AppendUInt(), or AppendUID().
I don't think we can change the string classes, unfortunately. AllocateImapUidString doesn't use AppendInt - it uses AppendUID, at least in the code in CVS that I'm looking at...but that's just the trunk code. I see what happened - the AllocateImapUidString code moved between the trunk and the branch, and the branch version doesn't use AppendUID - I'll fix that, but I don't know when it will make its way into a release... It would be helpful if you could apply the patch I'm about to generate to a clean 22.214.171.124 tree, and see if you still have any problems with negative uid's (i.e., remove your existing patch).
Created attachment 289544 [details] [diff] [review] fix AllocateImapUIDString
Attachment #289544 - Flags: superreview?(mscott)
Hi David, I'll try to get to it over the weekend. Lars
Attachment #289544 - Flags: superreview?(mscott) → superreview+
Lars, thx. If it works for you, I'll request approval to check this in for the next 2.0.0.x release. Or, if you can give me a test account on your server with the negative UID's, I can try it myself :-)
So I'm a complete C++ n00b, but I've been blindly poking around, and I noticed fUidOfSingleMessageFetch is defined in /mailnews/imap/src/nsImapServerResponseParser.h as PRInt32 instead of PRUint32. It appears this doesn't actually get used for anything, so it should be irrelevant, but it would be good to fix, yes? I've found other things defined as signed integers that look like they probably should be unsigned, but so far I'm at a loss as to which variable is causing the problem here. Definitely still not working as of Thunderbird 126.96.36.199 though. I've set up a test account; e-mail me if you want the login info.
Created attachment 428830 [details] [diff] [review] cleanup unused var - checked in.
imap uids > 0x80000000 are working in 3.0, afaik. I do remember fixing a bug sometime during the 3.0 process, but I don't remember the number.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
Resolution: WORKSFORME → DUPLICATE
Duplicate of bug: 518918
Attachment #428830 - Attachment description: cleanup unused var → cleanup unused var - checked in.
You need to log in before you can comment on or make changes to this bug.