The default bug view has changed. See this FAQ.

nsFileSpec::Truncate() expects PRInt32 as the first argument so mail may disappear.

RESOLVED FIXED in mozilla1.9.1b2

Status

()

Core
XPCOM
--
critical
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: hiro, Assigned: hiro)

Tracking

({dataloss, fixed1.8.1.18})

1.8 Branch
mozilla1.9.1b2
dataloss, fixed1.8.1.18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
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.
(Assignee)

Updated

9 years ago
Version: unspecified → 2.0
(Assignee)

Comment 1

9 years ago
Created attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.
(Assignee)

Updated

9 years ago
Component: General → XPCOM
Product: Thunderbird → Core
Version: 2.0 → 1.8 Branch
(Assignee)

Comment 2

9 years ago
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
(Assignee)

Comment 3

9 years ago
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}
QA Contact: general → xpcom
(Assignee)

Comment 4

9 years ago
Set severity to critical because of user data loss.
Severity: normal → critical
(Assignee)

Comment 5

9 years ago
David, could you please take a look at the patch and nsParseMailbox.cpp in #2.

I think there is a risk of data loss.
(Assignee)

Comment 6

9 years ago
Created attachment 333674 [details] [diff] [review]
Patch for Tell()
Attachment #333674 - Flags: review?(bienvenu)
(Assignee)

Updated

9 years ago
Keywords: dataloss

Comment 7

9 years ago
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?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 8

9 years ago
(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.


(Assignee)

Comment 9

9 years ago
I guess the person commented in https://bugzilla.mozilla.org/show_bug.cgi?id=321371#c69 faced to this issue.
(Assignee)

Comment 10

9 years ago
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.

(Assignee)

Comment 11

9 years ago
With patches in https://bugzilla.mozilla.org/show_bug.cgi?id=450698 and this bug, Thunderbird can receive test mail correctly.

(Assignee)

Updated

9 years ago
Attachment #333512 - Flags: review?(shaver)
(Assignee)

Comment 12

9 years ago
Comment on attachment 333674 [details] [diff] [review]
Patch for Tell()

The patch raises stability and robustness.
Attachment #333674 - Flags: approval1.8.1.17?
(Assignee)

Comment 13

9 years ago
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

The patch raises stability and robustness.
Attachment #333512 - Flags: approval1.8.1.17?
(Assignee)

Comment 14

9 years ago
Created attachment 334209 [details] [diff] [review]
Revised patch for Tell().

I am sorry, my previous patch has typo.

Fix PRUInt32 -> PRUint32.
Attachment #333674 - Attachment is obsolete: true
Attachment #334209 - Flags: superreview?(shaver)
Attachment #334209 - Flags: review?(bienvenu)
Attachment #334209 - Flags: approval1.8.1.17?
Attachment #333674 - Flags: review?(bienvenu)
Attachment #333674 - Flags: approval1.8.1.17?
Setting dependency of Bug 321371, according to Comment #9.
Blocks: 449741

Comment 16

9 years ago
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...
Attachment #334209 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 17

9 years ago
Created attachment 334222 [details] [diff] [review]
[checked in on 1.8] Revided patch following comment #16
Attachment #334209 - Attachment is obsolete: true
Attachment #334222 - Flags: superreview?(shaver)
Attachment #334222 - Flags: review+
Attachment #334222 - Flags: approval1.8.1.17?
Attachment #334209 - Flags: superreview?(shaver)
Attachment #334209 - Flags: approval1.8.1.17?
(Assignee)

Comment 18

9 years ago
(In reply to comment #17)
> Created an attachment (id=334222) [details]
> Revided patch following comment #16

Oops! I meant "Revised" of course.

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 to avoid misleading/confusion, for ease of track)
Problem reported by Comment #10 is Bug 449741.
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Please get all reviews before requesting check-in approval
Attachment #333512 - Flags: approval1.8.1.17?
Attachment #334222 - Flags: approval1.8.1.17?

Comment 22

9 years ago
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 :-)
Attachment #334222 - Flags: superreview?(shaver) → superreview?(neil)
Aw, man, I was writing an aria about my feelings for nsFileSpec!

Thanks, though. :)

Updated

9 years ago
Attachment #334222 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 24

9 years ago
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

Neil, could you please review the patch  for Truncate()?
Attachment #333512 - Flags: superreview?(neil)
Attachment #333512 - Flags: review?(shaver)
Attachment #333512 - Flags: review?(neil)
(Assignee)

Updated

9 years ago
Attachment #334222 - Flags: approval1.8.1.17?

Updated

9 years ago
Attachment #333512 - Flags: superreview?(neil)
Attachment #333512 - Flags: superreview+
Attachment #333512 - Flags: review?(neil)
Attachment #333512 - Flags: review?(bienvenu)

Comment 25

9 years ago
Comment on attachment 333512 [details] [diff] [review]
[checked in on branch] Patch for stable branch.

thx for the patch
Attachment #333512 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

9 years ago
Attachment #333512 - Flags: approval1.8.1.17?
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.
Attachment #333512 - Flags: approval1.8.1.17? → approval1.8.1.17+
Attachment #334222 - Flags: approval1.8.1.17? → approval1.8.1.17+
Assignee: nobody → poincare
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [checkin-needed: 1.8 branch]
Attachment #333512 - Flags: approval1.8.1.18?
Attachment #333512 - Flags: approval1.8.1.17-
Attachment #333512 - Flags: approval1.8.1.17+
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.
Attachment #334222 - Flags: approval1.8.1.18?
Attachment #334222 - Flags: approval1.8.1.17-
Attachment #334222 - Flags: approval1.8.1.17+

Updated

9 years ago
Keywords: checkin-needed
Whiteboard: [checkin-needed: 1.8 branch]
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
Attachment #333512 - Flags: approval1.8.1.18? → approval1.8.1.18+
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
Attachment #334222 - Flags: approval1.8.1.18? → approval1.8.1.18+
Keywords: checkin-needed
None of these patches are for the trunk, correct?

Comment 31

9 years ago
correct, we're not using nsFileSpec on the trunk
(In reply to comment #31)
> correct, we're not using nsFileSpec on the trunk

If it's not being used, can it be removed?
Duplicate of this bug: 278171
(In reply to comment #32)
> If it's not being used, can it be removed?

Bug 38122 ;-)
Status: NEW → ASSIGNED
Hardware: PC → All
Whiteboard: [c-n: 1.8.1 branch only, 2 patches]

Comment 35

9 years ago
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
Attachment #333512 - Attachment description: Patch for stable branch. → [checked in on branch] Patch for stable branch.

Comment 36

9 years ago
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
Attachment #334222 - Attachment description: Revided patch following comment #16 → [checked in on 1.8] Revided patch following comment #16

Updated

9 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed → fixed1.8.1.18
Resolution: --- → FIXED
Whiteboard: [c-n: 1.8.1 branch only, 2 patches]
Target Milestone: --- → mozilla1.9.1b2

Updated

8 years ago
Depends on: 494706
You need to log in before you can comment on or make changes to this bug.