Last Comment Bug 450359 - nsFileSpec::Truncate() expects PRInt32 as the first argument so mail may disappear.
: nsFileSpec::Truncate() expects PRInt32 as the first argument so mail may disa...
Status: RESOLVED FIXED
: dataloss, fixed1.8.1.18
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: 1.8 Branch
: All All
: -- critical (vote)
: mozilla1.9.1b2
Assigned To: Hiroyuki Ikezoe (:hiro)
:
Mentors:
: 278171 (view as bug list)
Depends on: 494706
Blocks: 449741
  Show dependency treegraph
 
Reported: 2008-08-12 23:01 PDT by Hiroyuki Ikezoe (:hiro)
Modified: 2009-06-16 22:37 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
[checked in on branch] Patch for stable branch. (9.41 KB, patch)
2008-08-12 23:05 PDT, Hiroyuki Ikezoe (:hiro)
mozilla: review+
neil: superreview+
samuel.sidler+old: approval1.8.1.17-
dveditz: approval1.8.1.18+
Details | Diff | Splinter Review
Patch for Tell() (5.02 KB, patch)
2008-08-13 19:08 PDT, Hiroyuki Ikezoe (:hiro)
no flags Details | Diff | Splinter Review
Revised patch for Tell(). (5.02 KB, patch)
2008-08-17 19:06 PDT, Hiroyuki Ikezoe (:hiro)
mozilla: review+
Details | Diff | Splinter Review
[checked in on 1.8] Revided patch following comment #16 (9.51 KB, patch)
2008-08-17 22:17 PDT, Hiroyuki Ikezoe (:hiro)
hiikezoe: review+
neil: superreview+
samuel.sidler+old: approval1.8.1.17-
dveditz: approval1.8.1.18+
Details | Diff | Splinter Review

Description Hiroyuki Ikezoe (:hiro) 2008-08-12 23:01:56 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.15) Gecko/20080702 Ubuntu/8.04 (hardy) Firefox/2.0.0.15 Kazehakase/0.5.4
Build Identifier: 

Thunderbird can handle mailbox below 4GB, but nsFileSpec::Truncate() expects PRInt32 as the first argument so some mails may disappear if the mail box size is over 2GB.



Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Comment 1 Hiroyuki Ikezoe (:hiro) 2008-08-12 23:05:37 PDT
Created attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.
Comment 2 Hiroyuki Ikezoe (:hiro) 2008-08-12 23:18:55 PDT
For example, the risk of disappearance at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/local/src/nsParseMailbox.cpp&rev=1.258.2.18#1676
Comment 3 Hiroyuki Ikezoe (:hiro) 2008-08-12 23:24:29 PDT
And Tell() should be expected PRUint32 too.

Index: xpcom/obsolete/nsFileSpecImpl.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/obsolete/nsFileSpecImpl.cpp,v
retrieving revision 1.7
diff -d -u -p -u -8 -p -r1.7 nsFileSpecImpl.cpp
--- xpcom/obsolete/nsFileSpecImpl.cpp   29 Jun 2005 22:23:55 -0000      1.7
+++ xpcom/obsolete/nsFileSpecImpl.cpp   13 Aug 2008 06:22:22 -0000
@@ -731,17 +731,17 @@ NS_IMETHODIMP nsFileSpecImpl::Seek(PRInt
                nsInputFileStream is(mInputStream);
                is.seek(offset);
                result = is.error();
        }
        return result;
 }

 //----------------------------------------------------------------------------------------
-NS_IMETHODIMP nsFileSpecImpl::Tell(PRInt32 *_retval)
+NS_IMETHODIMP nsFileSpecImpl::Tell(PRUint32 *_retval)
 //----------------------------------------------------------------------------------------
 {
        TEST_OUT_PTR(_retval)
        if (!mInputStream)
                return NS_ERROR_NULL_POINTER;
        nsInputFileStream s(mInputStream);
        *_retval = s.tell();
        return s.error();
Index: xpcom/obsolete/nsIFileSpec.idl
===================================================================
RCS file: /cvsroot/mozilla/xpcom/obsolete/nsIFileSpec.idl,v
retrieving revision 1.7
diff -d -u -p -u -8 -p -r1.7 nsIFileSpec.idl
--- xpcom/obsolete/nsIFileSpec.idl      29 Jun 2005 22:23:55 -0000      1.7
+++ xpcom/obsolete/nsIFileSpec.idl      13 Aug 2008 06:22:22 -0000
@@ -150,17 +150,17 @@ interface nsIFileSpec : nsISupports
     /** Check eof() before each call.
      * CAUTION: false result only indicates line was truncated
      * to fit buffer, or an error occurred (OTHER THAN eof).
      */
        long write(in string data, in long requestedCount);
        void flush();

        void seek(in long offset);
-       long tell();
+       unsigned long tell();
        void endLine();
        attribute AString unicodePath;

 };

 // Define Contractid and CID
 %{C++
 // {a3020981-2018-11d3-915f-a957795b7ebc}
Comment 4 Hiroyuki Ikezoe (:hiro) 2008-08-13 18:59:58 PDT
Set severity to critical because of user data loss.
Comment 5 Hiroyuki Ikezoe (:hiro) 2008-08-13 19:07:42 PDT
David, could you please take a look at the patch and nsParseMailbox.cpp in #2.

I think there is a risk of data loss.
Comment 6 Hiroyuki Ikezoe (:hiro) 2008-08-13 19:08:32 PDT
Created attachment 333674 [details] [diff] [review]
Patch for Tell()
Comment 7 David :Bienvenu 2008-08-13 21:47:00 PDT
Thx for the patch. We're not using nsIFileSpec on the trunk at all, so I don't think that part is needed...this patch is for the trunk, right?
Comment 8 Hiroyuki Ikezoe (:hiro) 2008-08-13 22:01:22 PDT
(In reply to comment #7)
> Thx for the patch. We're not using nsIFileSpec on the trunk at all, so I don't
> think that part is needed...this patch is for the trunk, right?

No, both patches for MOZILLA_1_8_BRANCH.


Comment 9 Hiroyuki Ikezoe (:hiro) 2008-08-14 18:15:22 PDT
I guess the person commented in https://bugzilla.mozilla.org/show_bug.cgi?id=321371#c69 faced to this issue.
Comment 10 Hiroyuki Ikezoe (:hiro) 2008-08-14 19:57:26 PDT
I confirmed breakage of mail folder.

Steps to reproduce

1. Create over 2GB Inbox. (I create it using dd command)
2. Disallow anti-virus clients to quarantine option.
3. Add a filter rule that subject contains test.
4. Send a mail with "test" subject from other mail client.
5. Receive the test mail by Thunderbird.

Result:

Thunderbird seemed to be giving up a process to filter the mail. I have no idea what was happened in Thunderbird but CPU usage of Thunderbird was 0 %.

Then I killed Thunderbird and restarted it, mails in the folder which is the target of the filtering were almost broken.

Comment 11 Hiroyuki Ikezoe (:hiro) 2008-08-14 19:59:01 PDT
With patches in https://bugzilla.mozilla.org/show_bug.cgi?id=450698 and this bug, Thunderbird can receive test mail correctly.

Comment 12 Hiroyuki Ikezoe (:hiro) 2008-08-17 18:24:05 PDT
Comment on attachment 333674 [details] [diff] [review]
Patch for Tell()

The patch raises stability and robustness.
Comment 13 Hiroyuki Ikezoe (:hiro) 2008-08-17 18:24:26 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

The patch raises stability and robustness.
Comment 14 Hiroyuki Ikezoe (:hiro) 2008-08-17 19:06:45 PDT
Created attachment 334209 [details] [diff] [review]
Revised patch for Tell().

I am sorry, my previous patch has typo.

Fix PRUInt32 -> PRUint32.
Comment 15 WADA 2008-08-17 20:22:07 PDT
Setting dependency of Bug 321371, according to Comment #9.
Comment 16 David :Bienvenu 2008-08-17 21:49:09 PDT
Comment on attachment 334209 [details] [diff] [review]
Revised patch for Tell().

this looks OK, but I think there are other callers of tell() that should be changed as well...
Comment 17 Hiroyuki Ikezoe (:hiro) 2008-08-17 22:17:07 PDT
Created attachment 334222 [details] [diff] [review]
[checked in on 1.8] Revided patch following comment #16
Comment 18 Hiroyuki Ikezoe (:hiro) 2008-08-17 22:18:17 PDT
(In reply to comment #17)
> Created an attachment (id=334222) [details]
> Revided patch following comment #16

Oops! I meant "Revised" of course.

Comment 19 WADA 2008-08-18 23:04:39 PDT
Removed dependency of Bug 321371, because main issue of Bug 321371 was different.
(Only several comments refer to "greater than 2GB" in that bug. They seem to be problem of Bug 449741, instead of problem of Bug 321371.)
Comment 20 WADA 2008-08-18 23:08:56 PDT
(Comment to avoid misleading/confusion, for ease of track)
Problem reported by Comment #10 is Bug 449741.
Comment 21 Daniel Veditz [:dveditz] 2008-08-20 15:08:48 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Please get all reviews before requesting check-in approval
Comment 22 David :Bienvenu 2008-08-20 15:14:38 PDT
Comment on attachment 334222 [details] [diff] [review]
[checked in on 1.8] Revided patch following comment #16

Shaver's probably not too excited about reviewing this :-)
Comment 23 Mike Shaver (:shaver -- probably not reading bugmail closely) 2008-08-20 15:17:20 PDT
Aw, man, I was writing an aria about my feelings for nsFileSpec!

Thanks, though. :)
Comment 24 Hiroyuki Ikezoe (:hiro) 2008-08-20 16:45:17 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Neil, could you please review the patch  for Truncate()?
Comment 25 David :Bienvenu 2008-08-21 07:11:01 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

thx for the patch
Comment 26 Daniel Veditz [:dveditz] 2008-08-26 14:41:38 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Both patches approved for 1.8.1.17, a=dveditz for release-drivers.
Comment 27 Samuel Sidler (old account; do not CC) 2008-08-27 01:50:40 PDT
Comment on attachment 334222 [details] [diff] [review]
[checked in on 1.8] Revided patch following comment #16

This will have to get looked at in 1.8.1.18. We're frozen.
Comment 28 Daniel Veditz [:dveditz] 2008-09-22 11:40:21 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Approved for 1.8.1.18, a=dveditz for release-drivers
Comment 29 Daniel Veditz [:dveditz] 2008-09-22 11:40:30 PDT
Comment on attachment 334222 [details] [diff] [review]
[checked in on 1.8] Revided patch following comment #16

Approved for 1.8.1.18, a=dveditz for release-drivers
Comment 30 Reed Loden [:reed] (use needinfo?) 2008-09-30 21:08:18 PDT
None of these patches are for the trunk, correct?
Comment 31 David :Bienvenu 2008-09-30 21:29:34 PDT
correct, we're not using nsFileSpec on the trunk
Comment 32 Reed Loden [:reed] (use needinfo?) 2008-09-30 21:32:54 PDT
(In reply to comment #31)
> correct, we're not using nsFileSpec on the trunk

If it's not being used, can it be removed?
Comment 33 Serge Gautherie (:sgautherie) 2008-10-13 11:03:02 PDT
*** Bug 278171 has been marked as a duplicate of this bug. ***
Comment 34 Serge Gautherie (:sgautherie) 2008-10-13 11:11:46 PDT
(In reply to comment #32)
> If it's not being used, can it be removed?

Bug 38122 ;-)
Comment 35 Magnus Melin 2008-10-17 11:46:51 PDT
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Checking in xpcom/obsolete/nsFileSpec.h;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpec.h,v  <--  nsFileSpec.h
new revision: 1.11.4.1; previous revision: 1.11
done
Checking in xpcom/obsolete/nsFileSpecBeOS.cpp;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpecBeOS.cpp,v  <--  nsFileSpecBeOS.cpp
new revision: 1.3.28.1; previous revision: 1.3
done
Checking in xpcom/obsolete/nsFileSpecImpl.cpp;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpecImpl.cpp,v  <--  nsFileSpecImpl.cpp
new revision: 1.7.4.1; previous revision: 1.7
done
Checking in xpcom/obsolete/nsFileSpecMac.cpp;
/cvsroot/mozilla/xpcom/obsolete/Attic/nsFileSpecMac.cpp,v  <--  nsFileSpecMac.cpp
new revision: 1.3.28.1; previous revision: 1.3
done
Checking in xpcom/obsolete/nsFileSpecOS2.cpp;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpecOS2.cpp,v  <--  nsFileSpecOS2.cpp
new revision: 1.3.28.2; previous revision: 1.3.28.1
done
Checking in xpcom/obsolete/nsFileSpecUnix.cpp;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpecUnix.cpp,v  <--  nsFileSpecUnix.cpp
new revision: 1.9.8.2; previous revision: 1.9.8.1
done
Checking in xpcom/obsolete/nsFileSpecWin.cpp;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpecWin.cpp,v  <--  nsFileSpecWin.cpp
new revision: 1.10.12.3; previous revision: 1.10.12.2
done
Checking in xpcom/obsolete/nsIFileSpec.idl;
/cvsroot/mozilla/xpcom/obsolete/nsIFileSpec.idl,v  <--  nsIFileSpec.idl
new revision: 1.7.4.1; previous revision: 1.7
done
Comment 36 Magnus Melin 2008-10-17 12:18:05 PDT
Comment on attachment 334222 [details] [diff] [review]
[checked in on 1.8] Revided patch following comment #16

Checking in mailnews/addrbook/src/nsVCard.cpp;
/cvsroot/mozilla/mailnews/addrbook/src/nsVCard.cpp,v  <--  nsVCard.cpp
new revision: 1.4.4.5; previous revision: 1.4.4.4
done
Checking in mailnews/db/msgdb/src/nsMailDatabase.cpp;
/cvsroot/mozilla/mailnews/db/msgdb/src/nsMailDatabase.cpp,v  <--  nsMailDatabase.cpp
new revision: 1.118.8.5; previous revision: 1.118.8.4
done
Checking in mailnews/import/src/ImportOutFile.cpp;
/cvsroot/mozilla/mailnews/import/src/ImportOutFile.cpp,v  <--  ImportOutFile.cpp
new revision: 1.6.28.1; previous revision: 1.6
done
Checking in mailnews/import/text/src/nsTextAddress.cpp;
/cvsroot/mozilla/mailnews/import/text/src/nsTextAddress.cpp,v  <--  nsTextAddress.cpp
new revision: 1.47.2.1; previous revision: 1.47
done
Checking in mailnews/local/src/nsLocalMailFolder.cpp;
/cvsroot/mozilla/mailnews/local/src/nsLocalMailFolder.cpp,v  <--  nsLocalMailFolder.cpp
new revision: 1.506.2.27; previous revision: 1.506.2.26
done
Checking in mailnews/local/src/nsParseMailbox.cpp;
/cvsroot/mozilla/mailnews/local/src/nsParseMailbox.cpp,v  <--  nsParseMailbox.cpp
new revision: 1.258.2.20; previous revision: 1.258.2.19
done
Checking in mailnews/local/src/nsPop3Sink.h;
/cvsroot/mozilla/mailnews/local/src/nsPop3Sink.h,v  <--  nsPop3Sink.h
new revision: 1.21.10.1; previous revision: 1.21
done
Checking in xpcom/obsolete/nsFileSpecImpl.cpp;
/cvsroot/mozilla/xpcom/obsolete/nsFileSpecImpl.cpp,v  <--  nsFileSpecImpl.cpp
new revision: 1.7.4.2; previous revision: 1.7.4.1
done
Checking in xpcom/obsolete/nsIFileSpec.idl;
/cvsroot/mozilla/xpcom/obsolete/nsIFileSpec.idl,v  <--  nsIFileSpec.idl
new revision: 1.7.4.2; previous revision: 1.7.4.1
done

Note You need to log in before you can comment on or make changes to this bug.