Closed
Bug 17581
Opened 25 years ago
Closed 25 years ago
Use ToNewCString and pass/return char* damage non ASCII data.
Categories
(MailNews Core :: MIME, defect, P3)
MailNews Core
MIME
Tracking
(Not tracked)
RESOLVED
FIXED
M11
People
(Reporter: tao, Assigned: rhp)
Details
GOOD:
http://lxr.mozilla.org/mozilla/source/mailnews/local/src/nsLocalStringBundle.cpp#53
Please review the below segment of code to see if we need to use
ToNewUnicode() instead of ToCString() to retrieve the string buffer from
nsAutoString. Using ToCString() might result in truncation of the second byte
of a double-byte character. Using the Unicode variants is always safer than
the CString one. Please reference the code segment above for the proper use.
BAD:
http://lxr.mozilla.org/mozilla/source/mailnews/mime/cthandlers/vcard/mimevcrd.cpp#1997
http://lxr.mozilla.org/mozilla/source/mailnews/mime/src/mimemoz2.cpp#1279
Thanks
Summary: [DOGFOOD] Please use ToNewUnicode() to retrieve string buffer → Please use ToNewUnicode() to retrieve string buffer
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M20
Assignee | ||
Comment 2•25 years ago
|
||
I just looked at this and I don't know how we can make this change without a
substancial rewrite of libmime. If you change the return of char * to PRUnichar
in these cases, then the caller needs to deal with it at PRUnichar * and so on,
and so on....
I just can't see this happening any time soon.
- rhp
Hi, Naoki:
Per my discussion with Frank TAng, I 'll let you drive this issue.
Thanks,
Tao
Comment 4•25 years ago
|
||
libmime in SeaMonkey is a rewrite. I don't understand why we pass data as char*
instead of PRUnichar* in the 1st rewrite. Everyone else is using nsString or
PRUnichar* in interface already....
Updated•25 years ago
|
Summary: Please use ToNewUnicode() to retrieve string buffer → Use ToNewCString and pass/return char* damage non ASCII data.
Comment 5•25 years ago
|
||
Change "Please use ToNewUnicode() to retrieve string buffer" to "Use
ToNewCString and pass/return char* damage non ASCII data."
Assignee | ||
Comment 6•25 years ago
|
||
libmime in SeaMonkey is NOT a rewrite. I would like to see where things are
broken to see this bug "in action" instead of having an automated routine
generate bugs for me. I (as do the rest of client engineering) have more bugs
than I'll ever get to before we call it Dogfood/Beta/PR?/FCS or any other
acronym you would like to think of.
I APOLOGIZE IN ADVANCE FOR THE TONE OF THIS RESPONSE.
I would just appreciate it if people would take my word for it when I say that
a particular change is non-trivial. I've been responsible for libmime since 5.0
started so I know a thing or two about it by now.
- rhp
Comment 7•25 years ago
|
||
I looked at the code with tao. I think they can use UTF-8 like other parts of
libmime so unicode text can be passed without data loss through char*.
Once those text needs to be passed to UI (or whatever needs Unichar*) they can
be converted from UTF-8 to UCS-2 (PRUnichar*) using nsTextFormater.
We want to test the two cases mentioned by tao and may create separate bugs
which can be tested/verified easier.
MimeGetStringByIDREAL, I cannot find callers of this is this going to be used in
future?
VCardGetStringByID, this is used for any localizable string for vCard, correct?
Assignee | ||
Comment 8•25 years ago
|
||
After M11 is done, I am going to turn on the real String Bundle calls. For a
while, string bundles did not work no the necko thread, but that is fixed now.
- rhp
Assignee | ||
Updated•25 years ago
|
Target Milestone: M20 → M11
Assignee | ||
Comment 9•25 years ago
|
||
Naoki made a really good point that my brain seemed to forget. We deal with
UTF-8 for most of libmime. So, a conversion to UTF-8 is a good thing that will
work with "char *", then we convert to UCS-2 before we do quoting, etc...but in
the case of display only, we just have our friend the META tag set the charset
to UTF-8.
I have the real calls turned on in my tree with this fix and I hope to get it
checked in today.
- rhp
Assignee | ||
Updated•25 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 10•25 years ago
|
||
After the nice reminder from Naoki, this has been changed to use the UTF-8
output format.
- rhp
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•