Closed
Bug 450359
Opened 16 years ago
Closed 16 years ago
nsFileSpec::Truncate() expects PRInt32 as the first argument so mail may disappear.
Categories
(Core :: XPCOM, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1b2
People
(Reporter: hiro, Assigned: hiro)
References
Details
(Keywords: dataloss, fixed1.8.1.18)
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
Bienvenu
:
review+
neil
:
superreview+
samuel.sidler+old
:
approval1.8.1.17-
dveditz
:
approval1.8.1.18+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
hiro
:
review+
neil
:
superreview+
samuel.sidler+old
:
approval1.8.1.17-
dveditz
:
approval1.8.1.18+
|
Details | Diff | Splinter Review |
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•16 years ago
|
Version: unspecified → 2.0
Assignee | ||
Comment 1•16 years ago
|
||
Assignee | ||
Updated•16 years ago
|
Component: General → XPCOM
Product: Thunderbird → Core
Version: 2.0 → 1.8 Branch
Assignee | ||
Comment 2•16 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•16 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}
Updated•16 years ago
|
QA Contact: general → xpcom
Assignee | ||
Comment 4•16 years ago
|
||
Set severity to critical because of user data loss.
Severity: normal → critical
Assignee | ||
Comment 5•16 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•16 years ago
|
||
Attachment #333674 -
Flags: review?(bienvenu)
Comment 7•16 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•16 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•16 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•16 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•16 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•16 years ago
|
Attachment #333512 -
Flags: review?(shaver)
Assignee | ||
Comment 12•16 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•16 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•16 years ago
|
||
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?
Comment 15•16 years ago
|
||
Setting dependency of Bug 321371, according to Comment #9.
Comment 16•16 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•16 years ago
|
||
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•16 years ago
|
||
(In reply to comment #17)
> Created an attachment (id=334222) [details]
> Revided patch following comment #16
Oops! I meant "Revised" of course.
Comment 19•16 years ago
|
||
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•16 years ago
|
||
(Comment to avoid misleading/confusion, for ease of track)
Problem reported by Comment #10 is Bug 449741.
Comment 21•16 years ago
|
||
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?
Updated•16 years ago
|
Attachment #334222 -
Flags: approval1.8.1.17?
Comment 22•16 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•16 years ago
|
Attachment #334222 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 24•16 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•16 years ago
|
Attachment #334222 -
Flags: approval1.8.1.17?
Updated•16 years ago
|
Attachment #333512 -
Flags: superreview?(neil)
Attachment #333512 -
Flags: superreview+
Attachment #333512 -
Flags: review?(neil)
Attachment #333512 -
Flags: review?(bienvenu)
Comment 25•16 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•16 years ago
|
Attachment #333512 -
Flags: approval1.8.1.17?
Comment 26•16 years ago
|
||
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+
Updated•16 years ago
|
Attachment #334222 -
Flags: approval1.8.1.17? → approval1.8.1.17+
Updated•16 years ago
|
Assignee: nobody → poincare
Assignee | ||
Updated•16 years ago
|
Keywords: checkin-needed
Whiteboard: [checkin-needed: 1.8 branch]
Updated•16 years ago
|
Attachment #333512 -
Flags: approval1.8.1.18?
Attachment #333512 -
Flags: approval1.8.1.17-
Attachment #333512 -
Flags: approval1.8.1.17+
Comment 27•16 years ago
|
||
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•16 years ago
|
Keywords: checkin-needed
Whiteboard: [checkin-needed: 1.8 branch]
Comment 28•16 years ago
|
||
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 29•16 years ago
|
||
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+
Updated•16 years ago
|
Keywords: checkin-needed
Comment 30•16 years ago
|
||
None of these patches are for the trunk, correct?
Comment 31•16 years ago
|
||
correct, we're not using nsFileSpec on the trunk
Comment 32•16 years ago
|
||
(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 34•16 years ago
|
||
Status: NEW → ASSIGNED
Hardware: PC → All
Whiteboard: [c-n: 1.8.1 branch only, 2 patches]
Comment 35•16 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•16 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•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 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
You need to log in
before you can comment on or make changes to this bug.
Description
•