277905 - IMAP Message UID Greater 2099999999 not displayed

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Helmut Hartmann]

on linux thunderbird 1.0, mails with UID greater then 209999999 won't displayed
in the message pane. 
on windows, thunderbird works fine.
Mailserver was mercur messaging 2005.

thanks a lot

ha

Comment 1

[Helmut Hartmann]

Attachment: fetch log with UID problem

Comment 2

[Konstantin Buharov]

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=223942

Bug is really critical - no way to read messages from IMAP account.

FYI, GNUMail and sylpheed are also affected by this defect.
Outlook Express & KMail - NOT affected.

RFC3501 IMHO does not require UIDs to be signed....

Comment 3

[David :Bienvenu]

one of you says it works on windows and the other doesn't - my guess is that
there's no reason for it to be different on linux...can anyone get me a test
account on a server with UID's like this? And do you have your inbox configured
for offline use?

Comment 4

[Helmut Hartmann]

My Configuration:
Windows XP SP", Thunderbird 1.0
first Mailaccount without Offline usage --> works fine
second Mailaccount with offline usage --> works fine

Suse Linux 9.2, Thunderbird 1.0
first Mailacoount without offline usage --> it won't work!
second Mailaccount with offline usage --< it won't work

in my case, under windows i can see the mails with uid greater than 2...  -
under linux not!
when i change the uid manually on the server (less than 2....) - the message
appear under linux! the mail was stored on the mercur-server with uid.i8 or uid.i9

the mails from this bug are send to my imap account. i only can read this under
windows. my preferred usage is my linux notebook

Comment 5

[Konstantin Buharov]

Attachment: Log from Windows with UID problem

I have generated IMAP log from my Thunderbird-1.0/Windows.
It is attached.

Look for the massive SendData with negative UIDs

I have truncated log from 700K to 60K.
Truncated part contains lines much like the last ones in attached log.

Unfortunately, I'm not an administrator of this server - so I'm unable to
provide account :-(

Comment 6

[Konstantin Buharov]

I'm able to provide similar log from Mozilla-1.7/Linux if needed...

Comment 7

[Helmut Hartmann]

look into my log. the log contains no negative uid
from atrium software support you can recive an patch for the mercur mailserver,
which correct the problem with negative uid.

but, the problem that i described continued exist.

i can't give you an access to the mailserver. it is not availble to the web.

Comment 8

[David :Bienvenu]

Helmut, 2147483647 is 0x7fffffff and it doesn't correspond to any actual msg in
that folder on your server. So it's not negative, but it's suspiciously close to
being negative.

Comment 9

[Konstantin Buharov]

As for offline usage - on Linux it does not matter behavior was not changed -
does not work. I'll check it later on Windows. There is probability that offline
usage could help there - in that case there are two different defects here.
Anyway, mails are available for sequential number in mailbox (IMAP folder). Only
UID acces is broken.

Sample of coding errors:
- void nsImapProtocol::ProcessSelectedStateURL():

                  //Set the current uid in server state parser (in case it was
used for new mail msgs earlier).                
GetServerStateParser().SetCurrentResponseUID((PRUint32)atoi(messageIdString));

converting result of atoi to unsigned is meaningless - atoi simply does not
handle numbers greater that 0x7fffffff.

Comment 10

[David :Bienvenu]

that would explain why it might work on windows but not linux, I guess...I'll
see if this is a regression...

Comment 11

[Konstantin Buharov]

I found the reason, guys, this is a bad style:
From nsTString.h:
        /**
         * Append the given unsigned integer to this string
         */
      inline void AppendInt( PRUint32 aInteger, PRInt32 aRadix = kRadix10 )
        {
          AppendInt(PRInt32(aInteger), aRadix);
        }

What is this?

nsIMAPBodyShell::nsIMAPBodyShell(nsImapProtocol *protocolConnection, const char
*buf, PRUint32 UID, const char *folderName)
{
.......
  m_UID.AppendInt(UID);


Even if proper AppendInt will be choosen by C++ compiler (with unsigned first
argument) - it does not matter - second one with signed will be called :-(

Comment 12

[Konstantin Buharov]

PRInt64(aInteger) may help, but I'm not sure that this will work for all
platforms/compilers...

Comment 13

[Konstantin Buharov]

Attachment: Patch to fix IMAP UIDs issue (mozilla-1.7 sources)

I have attached patch that solves this issue for me.
Patch is against mozilla 1.7 sources and I have checked compilation on Linux
only at this time.
IMHO it should be appliable to the newest mozilla sources and thunderbird ones.

Comment 14

[David :Bienvenu]

thx for doing this, the imap changes look OK (though for checking in, we'd need
to remove all the commented out parts).

But I don't think the string changes are going to fly - cc'ing Darin for his
input...

Comment 15

[Konstantin Buharov]

I agree that changes to strings may affect another components...

To speedup things, you could try to use AppendInt(PRInt64...) variant.
In nsIMAPBodyShell methods change

m_UID.AppendInt(UID)

to 
m_UID.AppendInt(PRInt64(UID))

"long long" should be sufficient to hold correct "unsigned long".

Without some correction (either that or in string library), mails still remains
unavailable - Mozilla simply sends improper request to server.
'atoi's are on parsing server replies path...

And please do not forget in first hunk of patch change "unsigned int" to the
proper type. Initially, int32 was here and I have no time to find unsigned
variant for it.

Comment 16

[David :Bienvenu]

*** Bug 278790 has been marked as a duplicate of this bug. ***

Comment 17

[Roger Sayle]

Attachment: Alternate patch for negative UID issue

The attached patch is a less intrusive fix to the problem.  Rather than modify
or extend the AppendInt methods of the shared nsCString class infrastructure, a local fix to mailnews is to use a local function to corectly construct/render UIDs for IMAP requests (using %u instead of %d).

p.s. I've not seen it mentioned in the bugzilla PR but UIDs greater than 0x80000000 are correctly handled by Microsoft's Outlook Express, so my employer is currently recommending that I use that instead of Thunderbird 2.0.0.6 :-(

Comment 18

[Roger Sayle]

Attachment: Alternate patch for negative UID issue (take 2)

Unfortunately bugzilla@mozilla bug 278790 isn't a perfect duplicate of bug 277905.  The incorrect/inappropriate usage of AppendInt in 278790 is in nsImapProtocol.cpp instead of n277905's sImapMailFolder.cpp.  To solve both, I've revised my previous patch to move the new "AppendUid" function into nsImapUtils.cpp, added a prototype to nsImapUtils.h and then make use of it in both nsImapMailFolder.cpp and nsImapProtocol.cpp.

Comment 19

[Roger Sayle]

Attachment: A recent fetch log on windows with the UID problem

I found the log file generated with NSPR_LOG_MODULES=IMAP:5 on my machine easier to debug than the circa 2005 logs already attached to this PR.  Here are the last 20 lines.  Notice the server's last UID is 3679392107, which in hex is 0xDB4F116B (greater than 0x80000000) which when interpreted as signed appears
as -615575189.  This negative value appears in the "UID fetch" generated by the current version of thunderbird, on the following line, and the servers error message "BAD Error in IMAP command UID: Invalid UID messageset" appears a line or two later.  I hope this helps.

Comment 20

[David :Bienvenu]

Comment on attachment 279267 [details] [diff] [review]
Alternate patch for negative UID issue (take 2)

this looks good, thx! I'll tweak the whitespace a little (I prefer spaces after commas separating params) and land it on the trunk.

Comment 21

[David :Bienvenu]

Comment on attachment 279267 [details] [diff] [review]
Alternate patch for negative UID issue (take 2)

oops, this doesn't compile for me on windows...I'll look into it.

Comment 22

[David :Bienvenu]

Attachment: fix landed on trunk.

I just needed to add an include to get this to compile...

Comment 23

[David :Bienvenu]

thx for the patch, Roger!

Comment 24

[David :Bienvenu]

fixed on trunk

Comment 25

[David :Bienvenu]

Comment on attachment 279737 [details] [diff] [review]
fix landed on trunk.

would be nice for a 1.8.1 branch release

Comment 26

[David :Bienvenu]

re-opening to re-assign to me...

Comment 27

[David :Bienvenu]

marking fixed - I'll land on the branch tonight if I can...

Comment 28

[David :Bienvenu]

fixed for 1.8.1.7