Closed
Bug 28197
Opened 25 years ago
Closed 25 years ago
NS_NewURI damange non ASCII data
Categories
(Core :: Networking, defect, P3)
Core
Networking
Tracking
()
VERIFIED
FIXED
M15
People
(Reporter: ftang, Assigned: warrensomebody)
Details
Attachments
(1 file)
1.27 KB,
patch
|
Details | Diff | Splinter Review |
Currently, NS_NewURI have the nsString favor of interface but it damage the non-ASCII data when it call ToCString. 53 inline nsresult 54 NS_NewURI(nsIURI* *result, const nsString& spec, nsIURI* baseURI = nsnull) 55 { 56 // XXX if the string is unicode, GetBuffer() returns null. 57 // XXX we need a strategy to deal w/ unicode specs (if there should 58 // XXX even be such a thing) 59 char* specStr = spec.ToNewCString(); // this forces a single byte char* 60 if (specStr == nsnull) 61 warren 1.1 return NS_ERROR_OUT_OF_MEMORY; 62 nsresult rv = NS_NewURI(result, specStr, baseURI); 63 nsAllocator::Free(specStr); 64 return rv; 65 } Suggestion- Either remove the nsString favor to push the responsability out from Necko or call ToNewUTF8String to reserved Unicode data and let the protcol handler deal with it in individual protocol. I don't think this is the *final* soultion for bug 14155 and 28171. But current call to ToCString is definitely wrong. Change it to call ToNewUTF8String may not solve the whole issue, but it have no possiblility to make it worst.
I vote for fixing NS_NewURI. The call to ToNewCstring corrupts any non-ASCII data. Changing the call ToNewUTF8String will not change data handling for the ASCII case and will preserve the non-ASCII data. Frank, Isn't this just a 1-line change?
Assignee | ||
Updated•25 years ago
|
Target Milestone: M15
Assignee | ||
Comment 2•25 years ago
|
||
There are 2 evil places in the nsNetUtil.h header that do that.
Reporter | ||
Comment 3•25 years ago
|
||
This is a one liner and it is blocking QA finding more i18n problem.
Keywords: beta1
Comment 5•25 years ago
|
||
Comment 6•25 years ago
|
||
Can I get a review on this please?
Comment 7•25 years ago
|
||
whoops. we crossed paths. I'm marking this fixed. I also pulled the dead comment out above one of the ToNewCString() calls.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•