Closed
Bug 1486706
Opened 7 years ago
Closed 7 years ago
Audit callers of nsTSubstring::SetCapacity() for subsequent writing past the logical length of the string via BeginWriting() in c-c
Categories
(MailNews Core :: General, enhancement)
MailNews Core
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 63.0
People
(Reporter: hsivonen, Assigned: jorgk-bmo)
References
Details
Attachments
(1 file, 1 obsolete file)
1.70 KB,
patch
|
hsivonen
:
review+
|
Details | Diff | Splinter Review |
This is the c-c version of bug 1472113.
See bug 1484668 for the details and to-be-landed documentation in the patch. The reason why the documentation hasn't landed is that it refers to an API that's pending review in bug 1482828.
Please audit c-c to see if this pattern occurs:
1) Call SetCapacity() on a string
2) Use BeginWriting() to write past mLength of the string but staying within the capacity set in the first step.
3) Call SetLength() with the number of code units actually written.
If that pattern occurs, please change things so that it doesn't occur either by changing the call to SetCapacity() to SetLength() (if applicable given other calls to the string), by replacing the write via BeginWriting() with a call to Append() or by using the BulkWrite() API to be added in bug 1482828.
Reporter | ||
Comment 1•7 years ago
|
||
Please change these two SetCapacity calls to SetLength instead:
https://searchfox.org/comm-central/source/mailnews/base/util/nsMsgI18N.cpp#102
https://searchfox.org/comm-central/source/mailnews/base/util/nsMsgI18N.cpp#139
That's it!
Assignee | ||
Comment 2•7 years ago
|
||
Thanks for the heads-up, Henri. It's not quite clear to me which bug we're porting here. Can you please give me a number. Would current usage break at some stage?
Assignee | ||
Comment 3•7 years ago
|
||
Oops, spurious spaces removed.
Attachment #9006049 -
Attachment is obsolete: true
Attachment #9006049 -
Flags: review?(hsivonen)
Attachment #9006050 -
Flags: review?(hsivonen)
Reporter | ||
Comment 4•7 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #2)
> Thanks for the heads-up, Henri. It's not quite clear to me which bug we're
> porting here. Can you please give me a number. Would current usage break at
> some stage?
This is in preparation for bug 1487341 and bug 1486711. And yes, those would break current usage.
Reporter | ||
Comment 5•7 years ago
|
||
Comment on attachment 9006050 [details] [diff] [review]
1486706-setLength.patch (v1b)
Review of attachment 9006050 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks!
Attachment #9006050 -
Flags: review?(hsivonen) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5f30a26c656a
Prepare for bug 1487341: replace use of SetCapacity() with SetLength (). r=hsivonen
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•7 years ago
|
Target Milestone: --- → Thunderbird 63.0
You need to log in
before you can comment on or make changes to this bug.
Description
•